- This file was added.
|1||# RUN: yaml2obj %s -o %t|
|3||# RUN: llvm-objcopy --set-section-alignment .foo=4 --set-section-alignment .bar=0x5 \|
|4||# RUN: --set-section-alignment .baz=0 %t %t.2|
|5||# RUN: llvm-readobj --sections %t.2 | FileCheck --check-prefix=CHECK %s|
|7||# CHECK: Name: .foo|
|8||# CHECK: AddressAlignment:|
|9||# CHECK-SAME: 4|
|10||# CHECK: Name: .bar|
Same for all the -SAME in this file
rupprecht: Same for all the -SAME in this file
If I do not use -SAME, I'll have to enumerate all fields between Name and AddressAlignment, with -NEXT:. I think -SAME is clearer.
CHECK: AddressAlignment: 4 can match something like:
Name: foo AddressAlignment: 8 Name: bar AddressAlignment: 4
MaskRay: If I do not use `-SAME`, I'll have to enumerate all fields between Name and AddressAlignment…
+1 to keeping -SAME. I find it less readable if I have to read lots of irrelevant fields. Maybe indenting the number might make it more readable, but I'm not sure:
# CHECK: Name: .foo # CHECK: AddressAlignment: # CHECK-SAME: 4
jhenderson: +1 to keeping `-SAME`. I find it less readable if I have to read lots of irrelevant fields.
Ah, I see why this simplifies it now. I wasn't familiar with this FileCheck idiom -- maybe we should add this to the docs. I'll see if I can write up my understanding.
Now that I get it, there's a whole lot of other tests I think we can clean up :)
rupprecht: Ah, I see why this simplifies it now. I wasn't familiar with this FileCheck idiom -- maybe we…
|11||# CHECK: AddressAlignment:|
|12||# CHECK-SAME: 5|
|13||# CHECK: Name: .baz|
|14||# CHECK: AddressAlignment:|
|15||# CHECK-SAME: 0|
|17||## If a section is specified multiple times, the last wins.|
|18||# RUN: llvm-objcopy --set-section-alignment .foo=4 --set-section-alignment=.foo=7 %t %t.3|
|19||# RUN: llvm-readobj --sections %t.3 | FileCheck --check-prefix=MULTI %s|
|21||# MULTI: Name: .foo|
|22||# MULTI: AddressAlignment:|
|23||# MULTI-SAME: 7|
|25||# RUN: not llvm-objcopy --set-section-alignment=.foo %t /dev/null 2>&1 | \|
|26||# RUN: FileCheck --check-prefix=MISSING-EQUAL %s|
|27||# MISSING-EQUAL: error: bad format for --set-section-alignment: missing '='|
The number of spaces between ":" and "error" is different in these 4 tests. I'd suggest stick to a single space everywhere.
btw, should this one be a error? Tools sometimes allows overriding the options (e.g. linker), it does not seem to be a huge problem to set
grimar: The number of spaces between ":" and "error" is different in these 4 tests. I'd suggest stick…
Yes, they are aligned by error: .
I agree this does not have to be an error. I'll let the last option win.
MaskRay: > or to align this message if your aim was to have a vertical aligned "error: " lines Yes…
|29||# RUN: not llvm-objcopy --set-section-alignment==4 %t /dev/null 2>&1 | \|
|30||# RUN: FileCheck --check-prefix=MISSING-SECTION %s|
|31||# MISSING-SECTION: error: bad format for --set-section-alignment: missing section name|
|33||# RUN: not llvm-objcopy --set-section-alignment=.foo=bar %t /dev/null 2>&1 | \|
|34||# RUN: FileCheck --check-prefix=BAD-ALIGN %s|
|35||# BAD-ALIGN: error: bad alignment for --set-section-alignment: 'bar'|
What about a test case for where the section does not exist?
jhenderson: What about a test case for where the section does not exist?
|39|| Class: ELFCLASS64|
|40|| Data: ELFDATA2LSB|
What about adding a section with alignment != 0 and a test that shows it can be dropped to 0?
grimar: What about adding a section with alignment != 0 and a test that shows it can be dropped to 0?
Also add a test for a very large alignment, e.g. 2^33?
rupprecht: Also add a test for a very large alignment, e.g. 2^33?
Huge alignment like --set-section-alignment=.foo=0x100000001 cannot be used in portable tests. The llvm-objcopy segment/section writer will advance file offsets to a multiple of the alignment, which will create a huge sparse file. If the underlying file system does not support sparse files, we may get ENOSPC.
(As an missing optimization, we can change the layout algorithm to not advance file offsets for the next section of SHT_NOBITS (GNU objcopy has such a rule), but I don't think that is worthwhile: we would lose the nice property that sh_offset is monotonically non-decreasing)
Given the file size problem, I do not want to test the 2^33 case.
MaskRay: Huge alignment like `--set-section-alignment=.foo=0x100000001` cannot be used in portable tests.
SGTM, can you just check manually that it works? I kinda want to just make sure that the getAsInteger method you're using won't silently fail for larger than int32 types.
If you had error checking for pow-2 types, you could also cheat by testing the error string since that would fail fast & not actually start to lay anything out in a file system (and hit the portability problems).
rupprecht: SGTM, can you just check manually that it works? I kinda want to just make sure that the…
|41|| Type: ET_REL|
|42|| Machine: EM_X86_64|
|44|| - Name: .foo|
|45|| Type: SHT_PROGBITS|
|46|| - Name: .bar|
|47|| Type: SHT_NOBITS|
|48|| - Name: .baz|
|49|| Type: SHT_NOTE|
|50|| AddressAlign: 4|