Allow "[[FLAGS=<none>]]" value in the ELF Fileheader Flags field #143845
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add amdgpu support in cross-arch-headers.test instead of its own test
jh7370
reviewed
Jun 12, 2025
There was a problem hiding this comment.
You need a dedicated yaml2obj test that shows that the new behaviour works as expected, i.e. if the FLAGS variable isn't defined, the e_flags value is 0, otherwise it's the specified value.
When I experimented with this locally, I had to change e_flags to std::optional and modify the mapOptional call that sets the Flags variable, but you may not have to do that. Without the test though, you can't tell!
Also, do not print Flags value when e_flags is 0
jh7370
reviewed
Jul 9, 2025
Add an obj2yaml eflags test
jh7370
reviewed
Jul 17, 2025
Comment on lines
1
to
3
| ## Test for FileHeader Flags | ||
| ## When FLAGS variable isn't defined, the e_flags value is 0, | ||
| ## otherwise it's the specified value |
There was a problem hiding this comment.
Suggested change
| ## Test for FileHeader Flags | |
| ## When FLAGS variable isn't defined, the e_flags value is 0, | |
| ## otherwise it's the specified value | |
| ## Test for FileHeader Flags. | |
| ## When FLAGS variable isn't defined, the e_flags value is 0. | |
| ## Otherwise, it's the specified value. |
There was a problem hiding this comment.
Please don't resolve conversations that I started. I will resolve them when I'm happy you've addressed my request.
aakanksha555
commented
Jul 17, 2025
| Class: ELFCLASS32 | ||
| Data: ELFDATA2LSB | ||
| Type: ET_EXEC | ||
| Machine: EM_ARM |
There was a problem hiding this comment.
Should I use a different target for this test too?
jh7370
reviewed
Jul 18, 2025
Comment on lines
1
to
3
| ## Test for FileHeader Flags | ||
| ## When FLAGS variable isn't defined, the e_flags value is 0, | ||
| ## Otherwise it's the specified value |
There was a problem hiding this comment.
You missed my other requests.
Suggested change
| ## Test for FileHeader Flags | |
| ## When FLAGS variable isn't defined, the e_flags value is 0, | |
| ## Otherwise it's the specified value | |
| ## Test for FileHeader Flags. | |
| ## When FLAGS variable isn't defined, the e_flags value is 0. | |
| ## Otherwise, it's the specified value |
Comment on lines
1
to
3
| ## Test for FileHeader Flags | ||
| ## When FLAGS variable isn't defined, the e_flags value is 0, | ||
| ## otherwise it's the specified value |
There was a problem hiding this comment.
Please don't resolve conversations that I started. I will resolve them when I'm happy you've addressed my request.
jh7370
approved these changes
Jul 21, 2025
Ya I think so. Let me squash and merge! |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/185/builds/22524 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/137/builds/22736 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/144/builds/30811 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/175/builds/22669 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/33/builds/20652 Here is the relevant piece of the build log for the reference |
|
Hi there, it seems this PR caused several tests failed that breaks our bots. Could you please take a look? Thanks! bot: https://lab.llvm.org/buildbot/#/builders/140/builds/27368 |
|
Reverted in b80ce05 for now. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/38806 Here is the relevant piece of the build log for the reference |
Thanks, looking! |
|
We are also see clang and lld test failures after this patch was landed. Looks like an assertion failure in yaml2obj: link to the build task: https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8708718394953370705/overview |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/56/builds/31389 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/60/builds/33470 Here is the relevant piece of the build log for the reference |
This was referenced Jul 23, 2025
mahesh-attarde
pushed a commit
to mahesh-attarde/llvm-project
that referenced
this pull request
Jul 28, 2025
…m#143845) llvm#92066 will be dependent on this change
mahesh-attarde
pushed a commit
to mahesh-attarde/llvm-project
that referenced
this pull request
Jul 28, 2025
…eld (llvm#143845)" This reverts commit 0b054e2. Breaks many tests, see comments on llvm#143845.
aakanksha555
added a commit
that referenced
this pull request
Jul 31, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#92066 will be dependent on this change