[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8299f261-7870-57a6-37cf-d4ce482ad81e@openwall.com>
Date: Sun, 19 Jan 2020 16:33:47 +0300
From: Alexander Cherepanov <ch3root@...nwall.com>
To: musl@...ts.openwall.com
Subject: Re: Minor style patch to exit.c
On 19/01/2020 14.31, Szabolcs Nagy wrote:
> * Markus Wichmann <nullplan@....net> [2020-01-19 12:07:43 +0100]:
>> The previous version did have a maze of parentheses here. But the actual
>> logic of the function is not to iterate over some numbers that happen to
>> be convertible to pointers, but rather to iterate over an array of
>> function pointers. So let us use pointer arithmetic correctly.
>> ---
>> src/exit/exit.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/exit/exit.c b/src/exit/exit.c
>> index a6869b37..e02ba2be 100644
>> --- a/src/exit/exit.c
>> +++ b/src/exit/exit.c
>> @@ -16,9 +16,9 @@ extern weak hidden void (*const __fini_array_start)(void), (*const __fini_array_
>>
>> static void libc_exit_fini(void)
>> {
>> - uintptr_t a = (uintptr_t)&__fini_array_end;
>> - for (; a>(uintptr_t)&__fini_array_start; a-=sizeof(void(*)()))
>> - (*(void (**)())(a-sizeof(void(*)())))();
>> + void (*const *a)() = &__fini_array_end;
>> + while (a > &__fini_array_start)
>> + (*--a)();
[This is part of my reply was sent to Markus off-list by mistake, sorry
for the dup.]
Iteration over pointers like this is wrong C (even if it's somewhat
disputed) and was miscompiled by gcc at some point -- for example you
can follow links from
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61502#c19 (see also the
next comment there for a reply from a gcc developer).
> this has undefined behaviour.
>
> the original code was carefully written to avoid that.
> you need to keep the uintptr_t cast since -- and > are
> undefined for pointers that go out of bound or don't
> point to the same object (and the _start, _end symbols
> don't represent the same c language object, they are
> independent).
I think this only solves a part of the problem.
1. Casts from integers to pointers are implementation-defined and only
guaranteed to give sane results when those integers are in turn casted
from pointers. Arithmetic on integers between casts is outside any
guarantees. It would be funny if not for the next issue.
2. As is defined, not only _start and _end symbols represent different
objects, locations between them represent different objects too.
Starting with a pointer to one object and iterating over integers until
you get to another object doesn't give you a valid pointer to the second
object, at least in gcc -- see
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65752 .
Couldn't _start defined as an array? Then separate values could be
accessed simply as elements of this array. And casts to integers could
be limited to calculating the number of elements, the terminating value
or something.
Beware though, even if you get it perfect from the C POV gcc has some
bugs in this area, e.g. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=49330 .
--
Alexander Cherepanov
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
