* [PATCH] aarch64: Fix warnings during libgcc build
@ 2023-07-11 9:37 Florian Weimer
2023-07-11 14:54 ` Richard Earnshaw (lists)
0 siblings, 1 reply; 5+ messages in thread
From: Florian Weimer @ 2023-07-11 9:37 UTC (permalink / raw)
To: gcc-patches
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] aarch64: Fix warnings during libgcc build
2023-07-11 9:37 [PATCH] aarch64: Fix warnings during libgcc build Florian Weimer
@ 2023-07-11 14:54 ` Richard Earnshaw (lists)
2023-07-11 15:09 ` Richard Earnshaw (lists)
2023-07-11 15:20 ` Florian Weimer
0 siblings, 2 replies; 5+ messages in thread
From: Richard Earnshaw (lists) @ 2023-07-11 14:54 UTC (permalink / raw)
To: Florian Weimer, gcc-patches; +Cc: Szabolcs Nagy
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] aarch64: Fix warnings during libgcc build
2023-07-11 14:54 ` Richard Earnshaw (lists)
@ 2023-07-11 15:09 ` Richard Earnshaw (lists)
2023-07-11 15:20 ` Florian Weimer
1 sibling, 0 replies; 5+ messages in thread
From: Richard Earnshaw (lists) @ 2023-07-11 15:09 UTC (permalink / raw)
To: Florian Weimer, gcc-patches; +Cc: Szabolcs Nagy
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] aarch64: Fix warnings during libgcc build
2023-07-11 14:54 ` Richard Earnshaw (lists)
2023-07-11 15:09 ` Richard Earnshaw (lists)
@ 2023-07-11 15:20 ` Florian Weimer
2023-07-12 8:05 ` Szabolcs Nagy
1 sibling, 1 reply; 5+ messages in thread
From: Florian Weimer @ 2023-07-11 15:20 UTC (permalink / raw)
To: Richard Earnshaw (lists); +Cc: gcc-patches, Szabolcs Nagy
* 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] aarch64: Fix warnings during libgcc build
2023-07-11 15:20 ` Florian Weimer
@ 2023-07-12 8:05 ` Szabolcs Nagy
0 siblings, 0 replies; 5+ messages in thread
From: Szabolcs Nagy @ 2023-07-12 8:05 UTC (permalink / raw)
To: Florian Weimer, Richard Earnshaw (lists); +Cc: gcc-patches
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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-07-12 8:05 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-11 9:37 [PATCH] aarch64: Fix warnings during libgcc build Florian Weimer
2023-07-11 14:54 ` Richard Earnshaw (lists)
2023-07-11 15:09 ` Richard Earnshaw (lists)
2023-07-11 15:20 ` Florian Weimer
2023-07-12 8:05 ` Szabolcs Nagy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).