[Toybox] imgtec patch: Fix static linkage of toybox binary.

[Evgenii Stepanov]

Sorry, I did not look at the problem hard enough.
The real issue is interaction of this code with safestack
(http://clang.llvm.org/docs/SafeStack.html), which splits the stack in
2 disjoint memory regions. If the two variables are allocated on
different stacks, the comparison result is truly undefined.

I don't really understand what this code is tying to do. Is it
catching unlimited stack growth? Why does the comment speak about
heap?
Maybe we could use __builtin_frame_address(0) instead?


On Fri, May 6, 2016 at 9:42 PM, Rich Felker <dalias at libc.org> wrote:
> On Fri, May 06, 2016 at 10:52:21PM -0500, Rob Landley wrote:
>> On 05/06/2016 08:38 PM, enh wrote:
>> > On Fri, May 6, 2016 at 5:12 PM, Rob Landley <rob at landley.net> wrote:
>> >> On 05/06/2016 02:56 PM, enh wrote:
>> >>> On Thu, May 5, 2016 at 8:15 PM, Rob Landley <rob at landley.net> wrote:
>> >>>> Applied, and that fetch+cherry-pick thing _also_ seems to avoid a
>> >>>> gratuitous merge commit, which is very nice.
>> >>>
>> >>> it also has the happy side-effect (because you keep the gerrit
>> >>> change-id line) of appearing in the UI as if the originally uploaded
>> >>> change was merged when i do my command-line merge from github. so if i
>> >>> hadn't told the imgtec guy i was sending this patch upstream first, as
>> >>> far as he knows it just got submitted here.
>> >>>
>> >>> (i'll still keep pointing folks upstream though, because the community
>> >>> of those fiddling with toybox should be around upstream, not AOSP or
>> >>> whichever other downstream they happen to use personally.)
>> >>
>> >> I'm happy to make better use of git, so if you care about the history of
>> >> a specific commit being preserved I can do that again.
>> >
>> > not particularly. the main advantage for me is that it's less work to
>> > just send you the appropriate link and copy/paste git command than to
>> > cherrypick myself and git format-patch (when you're just going to have
>> > to do the same on your end anyway) :-)
>> >
>> > by strange coincidence, i have another one for you today: "Fix UB in
>> > stack depth calculation."
>> > (https://android-review.googlesource.com/223547)
>>
>> Except this is one of those "not taking the patch as-is" things, because
>> it's got a variable declaration in the middle of a block, and I'd like
>> an in-situ comment explaining why we do a non-obvious thing.
>>
>> Is typecasting both pointers to (long) insufficient here? (That's being
>> pretty darn explicit that I want to do math on the integer
>> representations of these pointers. I know compiler writers are crazy
>> these days, but how crazy are we talking?)
>>
>> If we need to explicitly copy it into a volatile long I can do that, but
>> I'd declare it at the top of the function...
>
> I don't see any need for the volatile object. Subtraction of the
> intptr_t's is perfectly valid and well-defined. And of course, since
> generally toybox code assumes long has the same width as intptr_t and
> avoids use of the latter, long would be more idiomatic here.
>
> Rich