feat: implement OUTPUT_FORMAT and OUTPUT_ARCH in linker scripts - #2295
Conversation
Signed-off-by: Vishruth Thimmaiah <vishruththimmaiah@gmail.com>
Signed-off-by: Vishruth Thimmaiah <vishruththimmaiah@gmail.com>
| .short("EB") | ||
| .help("Select the big-endian format in the OUTPUT_FORMAT command") | ||
| .execute(|args, _modifier_stack| { | ||
| args.output_format_endian = Some(Endianness::Big); |
There was a problem hiding this comment.
Accepting the flag -EB even though we don't support big-endian seems a little misleading. GNU ld does more than just switch which entry from OUPUT_FORMAT gets selected. That said, lld does appear to only use -EB to select OUTPUT_FORMAT entry, so I guess it's not too bad. I guess also if you use -EB and put a format that is actually big endian in that slot, then you'll get an error. OK, I think despite my initial skepticism of this, I'm actually OK with it, but I'll leave this comment here to document my thought process in case others have similar concerns.
| @@ -0,0 +1 @@ | |||
| OUTPUT_FORMAT(elf64-aarch64) | |||
There was a problem hiding this comment.
The three-argument tests use quoted form while the single argument tests use the unquoted form. From what I can gather, both quoted and unquoted are accepted. Should we test that we can also parse quoted three argument and unquoted single-argument? Doing so from an integration test might be excessive, so this could be a good spot for a unit test - i.e. just test the parsing. You could even just assert that parsing OUTPUT_FORMAT(elf32-i386) is equal to parsing OUTPUT_FORMAT("elf32-i386").
There was a problem hiding this comment.
Fixed, although ld appears to not treat a comma as a delimiter, so OUTPUT_FORMAT with 3 arguments wouldn't work if they are unquoted. lld works with both quoted or unquoted arguments.
Signed-off-by: Vishruth Thimmaiah <vishruththimmaiah@gmail.com>
OUTPUT_FORMATis parsed and emits a error for now if it does not match the output target set using command line flags/default host arch.OUTPUT_ARCHis parsed and ignored for now, matching what lld does(https://github.com/llvm/llvm-project/blob/main/lld/ELF/ScriptParser.cpp#L454).