Page MenuHomePhabricator

Fix stack address builtin for negative numbers

Authored by zoecarver on Aug 27 2019, 3:49 PM.



This patch checks that an argument passed to either of the builtins __builtin_frame_address or __builtin_return_address is at least 0. This patch resolves the issue where these builtins would cause an infinite loop ( 25497 ).

Diff Detail

Event Timeline

zoecarver created this revision.Aug 27 2019, 3:49 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 27 2019, 3:49 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
zoecarver marked an inline comment as done.
zoecarver added inline comments.

For some reason lit was still erroring here.

We usually prefer to generate error messages for incorrect parameters to builtins in SemaChecking.cpp.

I don't think there's really an infinite loop here. The parameter to __builtin_frame_address is an unsigned integer; values greater than 0x7FFFFFFU aren't special. The reason it takes a very long time is that we try to unroll the implied loop; generating an object file with four billion load instructions takes essentially forever. We could impose a smaller limit, but it would be sort of arbitrary.

Thanks for the explanation. Should I then not try to "fix" the issue? Or should I update sema checking?

In the context of __builtin_frame_address, an arbitrary limit is probably okay. Maybe something like 0xFFFF, which is larger than anyone would realistically use, but doesn't take a crazy amount of time to compile.

Jim added a subscriber: Jim.Dec 15 2019, 11:16 PM
Jim added a comment.Dec 16 2019, 1:53 AM

I think this checking should be implemented in lib/Sema/SemaChecking.cpp.

@Jim Thanks, I'll move it.

  • move verification to SemaChecking
This revision is now accepted and ready to land.Feb 24 2020, 12:49 PM
This revision is now accepted and ready to land.Feb 24 2020, 2:37 PM
zoecarver updated this revision to Diff 246486.Feb 25 2020, 9:06 AM
  • Error if SemaBuiltinConstantArgRange returns true, not false

I was using SemaBuiltinConstantArgRange incorrectly. I was returning an error for !SemaBuiltinConstantArgRange instead of SemaBuiltinConstantArgRange. Also, I only modified the fail tests so I wasn't able to catch the error. Updated and am running both the Sema and CodeGen tests now. Should fix the error.

zoecarver updated this revision to Diff 246488.Feb 25 2020, 9:09 AM
  • diff from master (arg phab...)
This revision was automatically updated to reflect the committed changes.