On Wed, Feb 5, 2025 at 4:40 PM Qing Zhao <qing.z...@oracle.com> wrote:
>
>
>
> > On Feb 5, 2025, at 07:46, Richard Biener <richard.guent...@gmail.com> wrote:
> >
> > On Tue, Feb 4, 2025 at 10:12 PM Qing Zhao <qing.z...@oracle.com> wrote:
> >>
> >> Hi,
> >>
> >> One of our big application segv in libgcc code while unwinding the stack.
> >>
> >> This is a random crash while the application throws a c++ exception and
> >> unwinds the stack. Incidents are random and never can be reproduced by any
> >> test case.
> >>
> >> The libgcc that is used is based on GCC 8.
> >>
> >> Fortunately, we got some information through the stack trace, and narrowed
> >> down the crash is in the routine "init_object" of libgcc/unwind-dw2-pde.c:
> >>
> >> 777 count = classify_object_over_fdes (ob, ob->u.single);
> >>
> >> when loading the 2nd parameter of the call to "classify_object_over_fdes".
> >> i.e, the address that is pointed by "ob->u.single" is invalid.
> >>
> >> And the outer caller we can track back is the following line 1066 of the
> >> routine
> >> "_Unwind_Find_FDE":
> >>
> >> 1060 /* Classify and search the objects we've not yet processed. */
> >> 1061 while ((ob = unseen_objects))
> >> 1062 {
> >> 1063 struct object **p;
> >> 1064
> >> 1065 unseen_objects = ob->next;
> >> 1066 f = search_object (ob, pc);
> >>
> >> Then we suspected that the construction of the static variable
> >> "unseen_objects"
> >> might have some bug.
> >>
> >> Then after studying the source code that constructs "unseen_objects" in
> >> libgcc/unwind-dw2-pde.c, I found an issue in the routines
> >> "__register_frame"
> >> and "__register_frame_table" as following:
> >>
> >> 127 void
> >> 128 __register_frame (void *begin)
> >> 129 {
> >> 130 struct object *ob;
> >> 131
> >> 132 /* If .eh_frame is empty, don't register at all. */
> >> 133 if (*(uword *) begin == 0)
> >> 134 return;
> >> 135
> >> 136 ob = malloc (sizeof (struct object));
> >> 137 __register_frame_info (begin, ob);
> >> 138 }
> >>
> >> 181 void
> >> 182 __register_frame_table (void *begin)
> >> 183 {
> >> 184 struct object *ob = malloc (sizeof (struct object));
> >> 185 __register_frame_info_table (begin, ob);
> >> 186 }
> >> 187
> >>
> >> In the above, one obvious issue in the source code is at line 136, line
> >> 137,
> >> and line 184 and line 185: the return value of call to "malloc" is not
> >> checked
> >> against NULL before it was passed as the second parameter of the follwoing
> >> call.
> >>
> >> This might cause unpredicted random behavior.
> >>
> >> I checked the latest trunk GCC libgcc, the same issue is there too.
> >>
> >> This patch is for the latest trunk GCC.
> >>
> >> Please let me know any comments on this?
> >
> > I think I've seen this elsewhere —
>
> Do you mean that you saw this problem (malloc return NULL during
> register_frame) previously?
It was probably reported to us (SUSE) from a customer/partner, also
from a (llvm?) JIT context.
> > the issue is the unwind register API does
> > not allow for failures but I also think calling abort() is bad.
>
> Okay, I see.
>
> >
> > Are you calling this from a JIT context or so?
>
> I will ask the bug submitter on this to make sure.
>
> > We're assuming that at program
> > start malloc() will not fail.
>
> So, the routine __register_frame is only called at the _start_ of a program?
> >
> > The proper solution is probably to add an alternate ABI which has a way to
> > fail
> > during registration.
>
> Any suggestion on this (or any other routine I can refer to?)
>
>
> Thanks a lot for the help.
>
> Qing
> >
> > Richard.
> >
> >> thanks.
> >>
> >> Qing
> >>
> >>
> >> =======================
> >>
> >> Fix two issues in libgcc/unwind-dw2-fde.c:
> >>
> >> A. The returned value of call to malloc is not checked against NULL.
> >> This might cause unpredicted behavior during stack unwinding.
> >>
> >> B. Check for null begin parameter (as well as pointer to null) in
> >> __register_frame_info_table_bases and __register_frame_table.
> >>
> >> libgcc/ChangeLog:
> >>
> >> * unwind-dw2-fde.c (__register_frame): Check the return value of
> >> call
> >> to malloc.
> >> (__register_frame_info_table_bases): Check for null begin parameter.
> >> (__register_frame_table): Check the return value of call to malloc,
> >> Check for null begin parameter.
> >> ---
> >> libgcc/unwind-dw2-fde.c | 12 ++++++++++++
> >> 1 file changed, 12 insertions(+)
> >>
> >> diff --git a/libgcc/unwind-dw2-fde.c b/libgcc/unwind-dw2-fde.c
> >> index cdfd3974c99..f0bc29d682a 100644
> >> --- a/libgcc/unwind-dw2-fde.c
> >> +++ b/libgcc/unwind-dw2-fde.c
> >> @@ -169,6 +169,8 @@ __register_frame (void *begin)
> >> return;
> >>
> >> ob = malloc (sizeof (struct object));
> >> + if (ob == NULL)
> >> + abort ();
> >> __register_frame_info (begin, ob);
> >> }
> >>
> >> @@ -180,6 +182,10 @@ void
> >> __register_frame_info_table_bases (void *begin, struct object *ob,
> >> void *tbase, void *dbase)
> >> {
> >> + /* If .eh_frame is empty, don't register at all. */
> >> + if ((const uword *) begin == 0 || *(const uword *) begin == 0)
> >> + return;
> >> +
> >> ob->pc_begin = (void *)-1;
> >> ob->tbase = tbase;
> >> ob->dbase = dbase;
> >> @@ -210,7 +216,13 @@ __register_frame_info_table (void *begin, struct
> >> object *ob)
> >> void
> >> __register_frame_table (void *begin)
> >> {
> >> + /* If .eh_frame is empty, don't register at all. */
> >> + if (*(uword *) begin == 0)
> >> + return;
> >> +
> >> struct object *ob = malloc (sizeof (struct object));
> >> + if (ob == NULL)
> >> + abort ();
> >> __register_frame_info_table (begin, ob);
> >> }
> >>
> >> --
> >> 2.31.1
>
>
