Page MenuHomePhabricator
Differential D137088 Diff 492285 llvm/test/tools/llvm-readobj/ELF/llvm-vs-json-format.test

Changeset View

Standalone View

View Options

llvm/test/tools/llvm-readobj/ELF/llvm-vs-json-format.test

  • This file was added.
# Check that the LLVM and JSON formats differ when their implementations diverge.
jhendersonUnsubmitted
Done
ReplyInline Actions

Having looked at this again, rather than a separate test showing the differences, I think I'd prefer the test cases being added to existing tests that test LLVM and GNU output styles. For the st_other case, that would be https://github.com/llvm/llvm-project/blob/main/llvm/test/tools/llvm-readobj/ELF/symbol-visibility.test (which is essentially the "main" st_other test) and potentially both https://github.com/llvm/llvm-project/blob/main/llvm/test/tools/llvm-readobj/ELF/aarch64-symbols-stother.test and https://github.com/llvm/llvm-project/blob/main/llvm/test/tools/llvm-readobj/ELF/mips-symbols-stother.test.

I was hoping we'd be able to share CHECK patterns, but of course, for JSON output, the keys are quoted, so that doesn't really work.

jhenderson: Having looked at this again, rather than a separate test showing the differences, I think I'd…
paulkirthAuthorUnsubmitted
Done
ReplyInline Actions

I'm a bit concerned that the number of tests and test cases will start to rapidly grow as we progress through this stack, since there is basically zero JSON testing as it stands, but let's see how this goes.

I'd also like to keep this test since it documents how the two formats differ in a single place, and subsequent patches build on top of it.

paulkirth: I'm a bit concerned that the number of tests and test cases will start to rapidly grow as we…
jhendersonUnsubmitted
Done
ReplyInline Actions

It's just showing what the other test cases show though, for example, the aarch test you've modified clearly shows the behaviour difference between the two for st_other fields.

If you think highlighting the difference in behaviour between the two is worthwhile (I'm not convinced that it is), it probably should be in the documentation, rather than duplicating test coverage.

Regarding the rapid test case growth, that's rather inevitable, I'd have thought: you've got a new(ish) file format, that hasn't been rigorously tested before. It clearly needs to be better tested, so it's rather unsurprising. Keeping this file around will just exacerbate the test growth, since it will become one giant soup of test cases, be hard to maintain, and be somewhat hard to read, I suspect going forwards.

jhenderson: It's just showing what the other test cases show though, for example, the aarch test you've…
# When the Other field is 0, the LLVM format prints the output differently,
# JSON formatting should always be consistent.
# RUN: yaml2obj %s -o %t.o
# RUN: llvm-readobj --symbols %t.o | FileCheck %s --check-prefix=LLVM
# RUN: llvm-readobj --symbols --pretty-print --elf-output-style=JSON %t.o | FileCheck %s --check-prefix=JSON
# LLVM: Name: foo
# LLVM: Other [ (0x80)
# LLVM-NEXT: STO_AARCH64_VARIANT_PCS (0x80)
# LLVM-NEXT: ]
# LLVM: Name: bar
# LLVM: Other: 0
# JSON: "Value": "foo",
# JSON: "Other": {
# JSON-NEXT: "RawFlags": 128,
# JSON-NEXT: "Flags": [
# JSON-NEXT: {
# JSON-NEXT: "Name": "STO_AARCH64_VARIANT_PCS",
# JSON-NEXT: "Value": 128
# JSON-NEXT: }
# JSON-NEXT: ]
# JSON-NEXT: },
# JSON: "Name": {
# JSON-NEXT: "Value": "bar",
# JSON: "Other": {
# JSON-NEXT: "RawFlags": 0,
# JSON-NEXT: "Flags": []
# JSON-NEXT: },
--- !ELF
FileHeader:
Class: ELFCLASS64
Data: ELFDATA2LSB
Type: ET_REL
Machine: EM_AARCH64
Symbols:
- Name: foo
Other: [ STO_AARCH64_VARIANT_PCS ]
- Name: bar
Other: [ ]