aarch64: Fix warnings during libgcc build
Checks
Commit Message
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(-)
Comments
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.
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.
* 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
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
>
@@ -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;
@@ -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