Address various shortcomings of current split-stack implementation.
Details
- Reviewers
ruiu • espindola grimar - Commits
- rG0f6d31812e1d: [LLD] Update split stack support to handle more generic prologues. Improve…
rL338750: [LLD] Update split stack support to handle more generic prologues. Improve…
rLLD338750: [LLD] Update split stack support to handle more generic prologues. Improve…
Diff Detail
- Repository
- rLLD LLVM Linker
- Build Status
Buildable 20933 Build 20933: arc lint + arc unit
Event Timeline
ELF/Arch/X86_64.cpp | ||
---|---|---|
490 | It seems now you can do check early: if (Loc + 8 >= End) return; // Replace "cmp %fs:0x70,%rsp" and subsequent branch // with "stc, nopl 0x0(%rax,%rax,1)" ... | |
test/ELF/x86-64-split-stack-prologue-adjust-fail.s | ||
9 | Could you check the whole error message here? (including filenames and symbol names you are printing). |
Update for comments.
Updating D49926: Update split stack support to handle more generic prologues.
Improve error handling.
Add test file for better code-coverage. Update tests to be more
complete.
ELF/InputSection.cpp | ||
---|---|---|
854 | Something is wrong here. You are reporting __more_stack_nonsplit symbol name (and in the test cases you are using the same name), | |
928 | I think we usually write like this: auto *IS = | |
test/ELF/x86-64-split-stack-prologue-adjust-fail.s | ||
9 | I think we do not rely on the test file name and usually use regexp for checking, example: # CHECK: error: {{.*}}.o:(.zdebug_str): decompress failed: | |
14 | Do you need the --defsym here? I would also rename UNDEFINED_MORE_STACK_NON_SPLIT so something a bit shorter. Maybe just ERROR? |
Thanks again. Hopefully this is the last round, but happy to make other changes.
test/ELF/x86-64-split-stack-prologue-adjust-fail.s | ||
---|---|---|
14 | Don't need the defsym, because this test is testing what happens when it has been omitted. Renamed to just ERROR. |
Yet another round.
Updating D49926: Update split stack support to handle more generic prologues.
Improve error handling.
Add test file for better code-coverage. Update tests to be more
complete.
Overall looking good.
ELF/Arch/X86_64.cpp | ||
---|---|---|
490 | This looks odd -- you passed a 5 byte string and then specify 4. |
Fix a typo.
Updating D49926: Update split stack support to handle more generic prologues.
Improve error handling.
Add test file for better code-coverage. Update tests to be more
complete.
LGTM.
test/ELF/x86-64-split-stack-prologue-adjust-fail.s | ||
---|---|---|
14 | I was mean you probably do not need passing --defsym __morestack=0x100 --defsym _start=0x300 here. |
It seems now you can do check early: