[PATCH] D38361: [ELF] Stop setting output section size early

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 30 03:33:56 PST 2017


jhenderson added a comment.

I was re-verifying this patch, prior to a ping, but I discovered that when applied, it highlighted a problem with https://reviews.llvm.org/rLLD318924, resulting in test failures with this patch. The problem is that that commit is another example of code relying on the section Size before it is finalized (and getting it wrong for some linker scripts cases). I have filed https://bugs.llvm.org/show_bug.cgi?id=35478 for it.

This made me wonder whether changing Size and OutSecOff to llvm::Optional type would bring some benefits. It would ensure that people cannot use the fields before they have been set for the first time (which with this patch would be in the first call to assignAddresses). The downside is the need to dereference it everywhere where it is used, so could cause quite a bit of code churn. Also, presumably tests would pick up any misuse after this point, so it might not be all that useful. Thoughts?


https://reviews.llvm.org/D38361





More information about the llvm-commits mailing list