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 <[email protected]> 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 <[email protected]> wrote: >> >> On 05/06/2016 02:56 PM, enh wrote: >> >>> On Thu, May 5, 2016 at 8:15 PM, Rob Landley <[email protected]> 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 _______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
