Skip to content

Conversation

@gareth-rees
Copy link
Member

Restore assembly code (removed in change 194595 / commit 76166b7) for spilling callee-save registers on FR and LI platforms.

Fixes job004158 (Register scanning approach is not reliable)

@gareth-rees gareth-rees force-pushed the branch/2020-09-01/regscan branch from b15a011 to 33ecd47 Compare September 1, 2020 10:21
@gareth-rees gareth-rees marked this pull request as ready for review September 1, 2020 12:04
@gareth-rees gareth-rees force-pushed the branch/2020-09-01/regscan branch 2 times, most recently from 8679109 to 4b9cea4 Compare September 1, 2020 13:21
Copy link
Member Author

@gareth-rees gareth-rees left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Need to update manual/source/topic/porting.rst too

This code was previously removed in change 194595 but the result was
not reliable as noted in job004158.
@gareth-rees gareth-rees force-pushed the branch/2020-09-01/regscan branch from 4b9cea4 to 3964391 Compare September 3, 2020 15:37
@gareth-rees
Copy link
Member Author

gareth-rees commented Dec 31, 2020

@UNAA008 @NickBarnes Any thoughts on this pull request? It has been open since June 2019, it would be nice to make some progress.

@rptb1
Copy link
Member

rptb1 commented Jan 10, 2021

In his original message, Filip Strömbäck writes:

When I compile the provided sample with -O3, the compiler does not use the rbp register for a frame pointer, but rather uses it to store local variables, in this case the variable "b".

I have a few questions.

  1. Are we sure this issue could apply to -O2 and below?
  2. Are we sure that design.stack-scan.sol.setjmp.justify is invalid by the standard?
  3. Do/can we support -O3 or even have a sensible policy about that any more?

I ask because removing assembler was quite a win, and we should not give it up lightly.

Relevant:

@fstromback
Copy link
Contributor

I just saw the latest answer to this PR, I'm sorry for being slow in getting back to you.

  1. I am fairly sure that the compiler utilizes ebp for other things even at -O2, or even lower levels. In a separate project (Pintos if you have seen it), I needed to explicitly add the -fno-omit-frame-pointer to avoid this issue, even if we do not run at any major optimization level. That was on x86 though, not x86-64 which has more registers. From your linked optimization levels, it seems like -fomit-frame-pointer is at -O1 in GCC.
  2. This observation was based on me digging around inside the implementation of setjmp and longjmp (it was either in libbacktrace or parts of the GCC standard library). This implementation relied on the DWARF unwind tables to recreate most information inside the jmp_buf, so it only needed to store the program counter in there essentially. From a quick search, it seems like the contents of jmp_buf is not specified.

I can investigate closer if you want. I just need to find the details I wrote down at the time.

…uild and test, and prepare for review and merge.
@rptb1
Copy link
Member

rptb1 commented Feb 13, 2023

See also #151 (comment)

@rptb1 rptb1 added the high risk This work is or would be of high risk of introducing defects. label Feb 20, 2023
@thejayps thejayps added the essential Will cause failure to meet customer requirements. Assign resources. label Feb 20, 2023
@rptb1 rptb1 linked an issue Feb 28, 2023 that may be closed by this pull request
@rptb1 rptb1 added os.fr Relates to FreeBSD os.li Relates to Linux labels Feb 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

essential Will cause failure to meet customer requirements. Assign resources. high risk This work is or would be of high risk of introducing defects. os.fr Relates to FreeBSD os.li Relates to Linux

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Register scanning approach is not reliable

5 participants