> On Feb 5, 2025, at 07:46, Richard Biener <[email protected]> wrote:
>
> On Tue, Feb 4, 2025 at 10:12 PM Qing Zhao <[email protected]> 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?
> 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
