-
Notifications
You must be signed in to change notification settings - Fork 40
Fix -1 e_flags with binary inputs #403
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
jonathonpenix
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @quic-areg!
|
ARM/AArch64 does not implement checkFlags |
lld seems to not zero flags for binary inputs for Hexagon, Arm, AArch64 either so that should be ok |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please zero the flags for all architectures, it does not make sense to not have the flags 0'ed for specific architectures.
$arm-none-eabi-ld.bfd -marmelf --format=binary mydata.blob -o mydata.elf && llvm-readelf -h mydata.elf | grep Flags
Flags: 0x0
$aarch64-linux-gnu-ld.bfd -maarch64elf --format=binary mydata.blob -o mydata.elf && llvm-readelf -h mydata.elf | grep Flags
Flags: 0x0
Please compare bfd behavior at all times
552b578 to
8c92cee
Compare
quic-seaswara
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of creating seperate commits this way, I would appreciate if you could seperate each target specific change to a seperate commit or a PR.
Combining all of them together is making it hard to review.
Binary files have no architecture flags, so the default value of -1 is used instead of 0. Also removes unused code in TargetInfo.h Signed-off-by: quic-areg <aregmi@quicinc.com>
Signed-off-by: quic-areg <aregmi@quicinc.com>
|
Please document this behavior in linker documentation |
parth-07
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Binary files have no architecture flags, so the
default value of -1 is used instead of 0.
Fixes #396