[PATCH] aarch64: Fix warnings during libgcc build

[PATCH] aarch64: Fix warnings during libgcc build

[Florian Weimer]

libgcc/

	* config/aarch64/aarch64-unwind.h (aarch64_cie_signed_with_b_key):
	Add missing const qualifier.  Cast from const unsigned char *
	to const char *.  Use __builtin_strchr to avoid an implicit
	function declaration.
	* config/aarch64/linux-unwind.h (aarch64_fallback_frame_state):
	Add missing cast.

---
 libgcc/config/aarch64/aarch64-unwind.h | 4 ++--
 libgcc/config/aarch64/linux-unwind.h   | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/libgcc/config/aarch64/aarch64-unwind.h b/libgcc/config/aarch64/aarch64-unwind.h
index 3ad2f8239ed..30e428862c4 100644
--- a/libgcc/config/aarch64/aarch64-unwind.h
+++ b/libgcc/config/aarch64/aarch64-unwind.h
@@ -40,8 +40,8 @@ aarch64_cie_signed_with_b_key (struct _Unwind_Context *context)
       const struct dwarf_cie *cie = get_cie (fde);
       if (cie != NULL)
 	{
-	  char *aug_str = cie->augmentation;
-	  return strchr (aug_str, 'B') == NULL ? 0 : 1;
+	  const char *aug_str = (const char *) cie->augmentation;
+	  return __builtin_strchr (aug_str, 'B') == NULL ? 0 : 1;
 	}
     }
   return 0;
diff --git a/libgcc/config/aarch64/linux-unwind.h b/libgcc/config/aarch64/linux-unwind.h
index 00eba866049..93da7a9537d 100644
--- a/libgcc/config/aarch64/linux-unwind.h
+++ b/libgcc/config/aarch64/linux-unwind.h
@@ -77,7 +77,7 @@ aarch64_fallback_frame_state (struct _Unwind_Context *context,
     }
 
   rt_ = context->cfa;
-  sc = &rt_->uc.uc_mcontext;
+  sc = (struct sigcontext *) &rt_->uc.uc_mcontext;
 
 /* This define duplicates the definition in aarch64.md */
 #define SP_REGNUM 31

Re: [PATCH] aarch64: Fix warnings during libgcc build

[Richard Earnshaw (lists)]

On 11/07/2023 10:37, Florian Weimer via Gcc-patches wrote:
> libgcc/
> 
> 	* config/aarch64/aarch64-unwind.h (aarch64_cie_signed_with_b_key):
> 	Add missing const qualifier.  Cast from const unsigned char *
> 	to const char *.  Use __builtin_strchr to avoid an implicit
> 	function declaration.
> 	* config/aarch64/linux-unwind.h (aarch64_fallback_frame_state):
> 	Add missing cast.
> 
> ---
> diff --git a/libgcc/config/aarch64/linux-unwind.h b/libgcc/config/aarch64/linux-unwind.h
> index 00eba866049..93da7a9537d 100644
> --- a/libgcc/config/aarch64/linux-unwind.h
> +++ b/libgcc/config/aarch64/linux-unwind.h
> @@ -77,7 +77,7 @@ aarch64_fallback_frame_state (struct _Unwind_Context *context,
>       }
>   
>     rt_ = context->cfa;
> -  sc = &rt_->uc.uc_mcontext;
> +  sc = (struct sigcontext *) &rt_->uc.uc_mcontext;
>   
>   /* This define duplicates the definition in aarch64.md */
>   #define SP_REGNUM 31
> 
> 

This looks somewhat dubious.  I'm not particularly familiar with the 
kernel headers, but a quick look suggests an mcontext_t is nothing like 
a sigcontext_t.  So isn't the cast just papering over some more 
fundamental problem?

R.

Re: [PATCH] aarch64: Fix warnings during libgcc build

[Richard Earnshaw (lists)]

On 11/07/2023 15:54, Richard Earnshaw (lists) via Gcc-patches wrote:
> On 11/07/2023 10:37, Florian Weimer via Gcc-patches wrote:
>> libgcc/
>>
>>     * config/aarch64/aarch64-unwind.h (aarch64_cie_signed_with_b_key):
>>     Add missing const qualifier.  Cast from const unsigned char *
>>     to const char *.  Use __builtin_strchr to avoid an implicit
>>     function declaration.
>>     * config/aarch64/linux-unwind.h (aarch64_fallback_frame_state):
>>     Add missing cast.
>>
>> ---
>> diff --git a/libgcc/config/aarch64/linux-unwind.h 
>> b/libgcc/config/aarch64/linux-unwind.h
>> index 00eba866049..93da7a9537d 100644
>> --- a/libgcc/config/aarch64/linux-unwind.h
>> +++ b/libgcc/config/aarch64/linux-unwind.h
>> @@ -77,7 +77,7 @@ aarch64_fallback_frame_state (struct _Unwind_Context 
>> *context,
>>       }
>>     rt_ = context->cfa;
>> -  sc = &rt_->uc.uc_mcontext;
>> +  sc = (struct sigcontext *) &rt_->uc.uc_mcontext;
>>   /* This define duplicates the definition in aarch64.md */
>>   #define SP_REGNUM 31
>>
>>
> 
> This looks somewhat dubious.  I'm not particularly familiar with the 
> kernel headers, but a quick look suggests an mcontext_t is nothing like 
> a sigcontext_t.  So isn't the cast just papering over some more 
> fundamental problem?
> 
> R.

Sorry, I was looking at the wrong set of headers.  It looks like these 
have to match. But in that case, I think we should have a comment about 
that here to explain the suspicious cast.

R.

Re: [PATCH] aarch64: Fix warnings during libgcc build

[Florian Weimer]

* Richard Earnshaw:

> On 11/07/2023 10:37, Florian Weimer via Gcc-patches wrote:
>> libgcc/
>> 	* config/aarch64/aarch64-unwind.h
>> (aarch64_cie_signed_with_b_key):
>> 	Add missing const qualifier.  Cast from const unsigned char *
>> 	to const char *.  Use __builtin_strchr to avoid an implicit
>> 	function declaration.
>> 	* config/aarch64/linux-unwind.h (aarch64_fallback_frame_state):
>> 	Add missing cast.
>> ---
>> diff --git a/libgcc/config/aarch64/linux-unwind.h b/libgcc/config/aarch64/linux-unwind.h
>> index 00eba866049..93da7a9537d 100644
>> --- a/libgcc/config/aarch64/linux-unwind.h
>> +++ b/libgcc/config/aarch64/linux-unwind.h
>> @@ -77,7 +77,7 @@ aarch64_fallback_frame_state (struct _Unwind_Context *context,
>>       }
>>       rt_ = context->cfa;
>> -  sc = &rt_->uc.uc_mcontext;
>> +  sc = (struct sigcontext *) &rt_->uc.uc_mcontext;
>>     /* This define duplicates the definition in aarch64.md */
>>   #define SP_REGNUM 31
>> 
>
> This looks somewhat dubious.  I'm not particularly familiar with the
> kernel headers, but a quick look suggests an mcontext_t is nothing
> like a sigcontext_t.  So isn't the cast just papering over some more
> fundamental problem?

I agree it looks dubious.  Note that it's struct sigcontext, not
(not-struct) sigcontext_t.  I don't know why the uc_mcontext members
aren't accessed directly, so I can't really write a good comment about
it.

Obviously it works quite well as-is. 8-)  Similar code is present in
many, many Linux targets.

Thanks,
Florian

Re: [PATCH] aarch64: Fix warnings during libgcc build

[Szabolcs Nagy]

The 07/11/2023 17:20, Florian Weimer wrote:
> * Richard Earnshaw:
> 
> > On 11/07/2023 10:37, Florian Weimer via Gcc-patches wrote:
> >> libgcc/
> >> 	* config/aarch64/aarch64-unwind.h
> >> (aarch64_cie_signed_with_b_key):
> >> 	Add missing const qualifier.  Cast from const unsigned char *
> >> 	to const char *.  Use __builtin_strchr to avoid an implicit
> >> 	function declaration.
> >> 	* config/aarch64/linux-unwind.h (aarch64_fallback_frame_state):
> >> 	Add missing cast.
> >> ---
> >> diff --git a/libgcc/config/aarch64/linux-unwind.h b/libgcc/config/aarch64/linux-unwind.h
> >> index 00eba866049..93da7a9537d 100644
> >> --- a/libgcc/config/aarch64/linux-unwind.h
> >> +++ b/libgcc/config/aarch64/linux-unwind.h
> >> @@ -77,7 +77,7 @@ aarch64_fallback_frame_state (struct _Unwind_Context *context,
> >>       }
> >>       rt_ = context->cfa;
> >> -  sc = &rt_->uc.uc_mcontext;
> >> +  sc = (struct sigcontext *) &rt_->uc.uc_mcontext;
> >>     /* This define duplicates the definition in aarch64.md */
> >>   #define SP_REGNUM 31
> >> 
> >
> > This looks somewhat dubious.  I'm not particularly familiar with the
> > kernel headers, but a quick look suggests an mcontext_t is nothing
> > like a sigcontext_t.  So isn't the cast just papering over some more
> > fundamental problem?
> 
> I agree it looks dubious.  Note that it's struct sigcontext, not
> (not-struct) sigcontext_t.  I don't know why the uc_mcontext members
> aren't accessed directly, so I can't really write a good comment about
> it.

historically glibc typedefed mcontext_t to linux struct sigcontext
so this used to work fine. (i dont know about other os-es)

then at some point glibc fixed the namespace polluting fields
when building for posix which required a separate mcontext_t.

i guess either fix works: moving to the correct mcontext_t or to
cast to struct sigcontext, but the former means the fields must
be changed when building in a posix conforming mode (i guess
libgcc is built with _GNU_SOURCE so may not be an issue) and
they may be different across different libcs (or even different
versions of glibc) then.

> 
> Obviously it works quite well as-is. 8-)  Similar code is present in
> many, many Linux targets.
> 
> Thanks,
> Florian
>