[llvm-objdump][test] Improve testing of some switches #2

[jhenderson]

Summary

This patch focuses on adding additional testing for the --source switch. For reference, the source-interleave-x86_64.ll has been split into two parts - the input (shared with the other tests) and the test itself.

[MaskRay]

test/tools/llvm-objdump/X86/Inputs/source-interleave.ll

2 ↗

(On Diff #199776)

Rename..

test/tools/llvm-objdump/X86/source-interleave-missing-source.test

5 ↗

(On Diff #199776)

I wanted to ask whether sed -e 's,SRC_COMPDIR,%p,' works on Windows, if it works, we don't need to use %/p.

In a *nix shell, printf %s '\a\b' => \a\b

[jhenderson]

test/tools/llvm-objdump/X86/Inputs/source-interleave.ll

2 ↗

(On Diff #199776)

Rename which bit? I can't rename the file to two different files (as things stand I've renamed X86/source-interleave-x86_64.ll to X86/source-interleave-x86_64.test). Also, my patch creation process may sometimes mess things up (sometimes it thinks things are modified, not added for example), but the final commit will actually be a rename.

If you're referring to renaming "source-interleave-x86_64.c" to something else, then no, because that needs to point at the .c file that this .ll was generated from (and that is unchanged).

test/tools/llvm-objdump/X86/source-interleave-missing-source.test

5 ↗

(On Diff #199776)

%p doesn't work in that string, because it becomes something like "C:\test\directory" and the \t and \d get interpreted as escape characters. See also https://reviews.llvm.org/D61856.

Note: I do most of my development on Windows, so I'll pick up issues like that.

[grimar]

I do not feel I am right person to accept that, would be nice to have someone else to take a look.
But I read carefully all the test cases and what is going on looks reasonable to me, so I think it is correct.
Minor question is inlined.

test/tools/llvm-objdump/X86/source-interleave-invalid-source.test

5 ↗

(On Diff #199776)

May be silly question, but what if it is lower? I.e. s,line: 7,line: -1,g?
Does it makes sence to test it?

[jhenderson]

test/tools/llvm-objdump/X86/source-interleave-invalid-source.test

5 ↗

(On Diff #199776)

I don't think so. That sounds like it's testing llc's handling of weird looking data. I'll experiment though to see what it actually ends up as.

[MaskRay]

test/tools/llvm-objdump/X86/Inputs/source-interleave.ll

2 ↗

(On Diff #199776)

Oh I thought you renamed source-interleave-x86_64.c to source-interleave.c. If the C source hasn't been renamed, why do you rename the .ll file?

17 ↗

(On Diff #199776)

I don't know how you feel about it, but I think these functions can just be empty. That is sufficient to emit debug info entries which will be used by the line table parser.

test/tools/llvm-objdump/X86/source-interleave-missing-source.test

5 ↗

(On Diff #199776)

That test said \t didn't work in double quotes. If \t doesn't work in single quotes, either, %/p seems fine.

[rupprecht]

test/tools/llvm-objdump/X86/Inputs/source-interleave.ll

17 ↗

(On Diff #199776)

I think having the real implementation (even if it's relatively short) will be a more realistic test of interleaved source. But maybe I don't understand your comment?

41 ↗

(On Diff #199776)

Mind adding a comment here (or maybe at the top) that tests using this file need to run sed to fix this? Took me longer than I would like to admit to realize that.

test/tools/llvm-objdump/X86/source-interleave-x86_64.test

1 ↗

(On Diff #199776)

Change ; to # for the test now that it doesn't have to parse as a .ll file

[MaskRay]

test/tools/llvm-objdump/X86/Inputs/source-interleave.ll

17 ↗

(On Diff #199776)

I ask because I find these irrelevant lines make the test hard to read. I am fine if you decide to keep the status quo.

What is important here is the source, the IR is just to provide debug information. If the function body is empty, we may argue that after optimizations the function is known to be empty.....

[jhenderson]

Address some review comments (discussed inline). Change a %t to %/t, and tidy up a couple of double spaces.

test/tools/llvm-objdump/X86/Inputs/source-interleave.ll

2 ↗

(On Diff #199776)

I can name it source-interleave-x86_64.ll if you prefer. It's in a different directory to where the code comes from, so it's still a new file.

17 ↗

(On Diff #199776)

I'm okay doing this, but maybe in a separate patch, since that keeps the diffs cleaner, and shouldn't impact the new tests. By the way, the original contributor was specifically asked to produce the .ll from the .c file for the original test.

41 ↗

(On Diff #199776)

I've added the comment to the top of the file.

test/tools/llvm-objdump/X86/source-interleave-invalid-source.test

5 ↗

(On Diff #199776)

As I suspected, llc emits an error if passed a negative number.

test/tools/llvm-objdump/X86/source-interleave-missing-source.test

5 ↗

(On Diff #199776)

I've tried single-quotes, and it doesn't work unfortunately.

[jhenderson]

@MaskRay, are you happy with this as is, or would you like any further changes in this patch?

[MaskRay]

I just wanted to ask if you want to make the test easier to read (making the function body empty), or to make it realistic (no change). I'm happy with either choice.

[jhenderson]

In D61996#1512299, @MaskRay wrote:

I just wanted to ask if you want to make the test easier to read (making the function body empty), or to make it realistic (no change). I'm happy with either choice.

I started looking at simplifying the test input, but quickly realised that I'm not familiar enough with the format, and couldn't get it to work easily. As there isn't a desperate need to change things, I'm inclined to leave it as is, although if others want to make that change, I'm not opposed to it.

[jhenderson]

This caused lit failures for somebody who had 'main' in their directory tree, so I am changing the --implicit-check-not=main lines into --implicit-check-not='main()'. Let me know if anybody thinks this is a bad idea.

[jhenderson]

In D61996#1515377, @jhenderson wrote:

This caused lit failures for somebody who had 'main' in their directory tree, so I am changing the --implicit-check-not=main lines into --implicit-check-not='main()'. Let me know if anybody thinks this is a bad idea.

Committed in rL361621.

llvm/trunk/test/tools/llvm-objdump/X86/Inputs/source-interleave.ll

+; NOTE: To be able to use this file as an input, the string SRC_CMPDIR needs
+;       replacing with a directory path by using sed or similar.
+
+; ModuleID = 'source-interleave-x86_64.bc'
+source_filename = "source-interleave-x86_64.c"
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+@a = global i32 1, align 4, !dbg !4
+
+; Function Attrs: nounwind uwtable
+define i32 @foo() #0 !dbg !9 {
+entry:
+  %0 = load i32, i32* @a, align 4, !dbg !12
+  ret i32 %0, !dbg !13
+}
+
+; Function Attrs: nounwind uwtable
+define i32 @main() #0 !dbg !14 {
+entry:
+  %retval = alloca i32, align 4
+  %b = alloca i32*, align 8
+  store i32 0, i32* %retval, align 4
+  call void @llvm.dbg.declare(metadata i32** %b, metadata !15, metadata !17), !dbg !18
+  store i32* @a, i32** %b, align 8, !dbg !18
+  %0 = load i32*, i32** %b, align 8, !dbg !19
+  %1 = load i32, i32* %0, align 4, !dbg !20
+  %call = call i32 @foo(), !dbg !21
+  %add = add nsw i32 %1, %call, !dbg !22
+  ret i32 %add, !dbg !23
+}
+
+; Function Attrs: nounwind readnone
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+
+attributes #0 = { nounwind uwtable "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { nounwind readnone }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!6, !7}
+!llvm.ident = !{!8}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 4.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, globals: !3)
+!1 = !DIFile(filename: "source-interleave-x86_64.c", directory: "SRC_COMPDIR")
+!2 = !{}
+!3 = !{!4}
+!4 = !DIGlobalVariableExpression(var: !DIGlobalVariable(name: "a", scope: !0, file: !1, line: 1, type: !5, isLocal: false, isDefinition: true), expr: !DIExpression())
+!5 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
+!6 = !{i32 2, !"Dwarf Version", i32 4}
+!7 = !{i32 2, !"Debug Info Version", i32 3}
+!8 = !{!"clang version 4.0.0"}
+!9 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 2, type: !10, isLocal: false, isDefinition: true, scopeLine: 2, isOptimized: false, unit: !0, retainedNodes: !2)
+!10 = !DISubroutineType(types: !11)
+!11 = !{!5}
+!12 = !DILocation(line: 3, column: 10, scope: !9)
+!13 = !DILocation(line: 3, column: 3, scope: !9)
+!14 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 6, type: !10, isLocal: false, isDefinition: true, scopeLine: 6, isOptimized: false, unit: !0, retainedNodes: !2)
+!15 = !DILocalVariable(name: "b", scope: !14, file: !1, line: 7, type: !16)
+!16 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !5, size: 64, align: 64)
+!17 = !DIExpression()
+!18 = !DILocation(line: 7, column: 8, scope: !14)
+!19 = !DILocation(line: 8, column: 11, scope: !14)
+!20 = !DILocation(line: 8, column: 10, scope: !14)
+!21 = !DILocation(line: 8, column: 15, scope: !14)
+!22 = !DILocation(line: 8, column: 13, scope: !14)
+!23 = !DILocation(line: 8, column: 3, scope: !14)

llvm/trunk/test/tools/llvm-objdump/X86/source-interleave-invalid-source.test

+## Test llvm-objdump's --source behaviour when a line number is greater than the
+## file length.
+
+# RUN: sed -e "s,SRC_COMPDIR,%/p/Inputs,g" %p/Inputs/source-interleave.ll > %t.ll
+# RUN: sed -e "s,line: 7,line: 9999,g" %t.ll > %t2.ll
+
+# RUN: llc -o %t.o -filetype=obj -mtriple=x86_64-pc-linux %t.ll
+# RUN: llc -o %t2.o -filetype=obj -mtriple=x86_64-pc-linux %t2.ll
+
+# RUN: llvm-objdump --source %t.o | FileCheck %s --check-prefixes=CHECK,GOOD
+# RUN: llvm-objdump --source %t2.o | FileCheck %s --implicit-check-not="int *b = &a;"
+
+# CHECK:      main:
+# CHECK-NEXT: ; int main() {
+# GOOD:       ;   int *b = &a;
+# CHECK:      ;   return *b + foo();

llvm/trunk/test/tools/llvm-objdump/X86/source-interleave-missing-source.test

+## Test that if the source cannot be found that disassembly is still printed,
+## and that no source is printed.
+
+# RUN: sed -e "s,SRC_COMPDIR,%/t,g" %p/Inputs/source-interleave.ll > %t.ll
+# RUN: sed -e "s,SRC_COMPDIR,%/p/Inputs,g" %p/Inputs/source-interleave.ll > %t2.ll
+
+# RUN: llc -o %t.o -filetype=obj -mtriple=x86_64-pc-linux %t.ll
+# RUN: llc -o %t2.o -filetype=obj -mtriple=x86_64-pc-linux %t2.ll
+
+# RUN: llvm-objdump --source %t.o | FileCheck %s --implicit-check-not=main
+# RUN: llvm-objdump --source %t2.o | FileCheck %s --check-prefixes=CHECK,SOURCE
+
+# CHECK:       0000000000000010 main:
+# SOURCE-NEXT: ; int main() {
+# CHECK-NEXT:   10:   55                      pushq   %rbp

llvm/trunk/test/tools/llvm-objdump/X86/source-interleave-no-debug-info.test

+## Test that if an object has no debug information, only the disassembly is
+## printed when --source is specified.
+
+# RUN: sed -e "s,SRC_COMPDIR,%/p/Inputs,g" %p/Inputs/source-interleave.ll > %t.ll
+# RUN: llc -o %t.o -filetype=obj -mtriple=x86_64-pc-linux %t.ll
+# RUN: llvm-objcopy --strip-debug %t.o %t2.o
+
+# RUN: llvm-objdump --source %t.o | FileCheck %s --check-prefixes=CHECK,SOURCE
+# RUN: llvm-objdump --source %t2.o | FileCheck %s --implicit-check-not=main
+
+# CHECK:       0000000000000010 main:
+# SOURCE-NEXT: ; int main() {
+# CHECK-NEXT:   10:   55                      pushq   %rbp

llvm/trunk/test/tools/llvm-objdump/X86/source-interleave-relative-paths.test

+## Test that llvm-objdump prints source and disassembly for relative directory
+## paths for appropriate working directories. We also test that when the directory
+## string is empty the behaviour is the same as the current working directory.
+
+# RUN: mkdir -p %t/a/b
+# RUN: cp %p/Inputs/source-interleave-x86_64.c %t/a/source-interleave-x86_64.c
+
+# RUN: sed -e "s,SRC_COMPDIR,a,g" %p/Inputs/source-interleave.ll > %t.ll
+# RUN: sed -e "s,SRC_COMPDIR,,g" %p/Inputs/source-interleave.ll > %t2.ll
+# RUN: sed -e "s,SRC_COMPDIR,.,g" %p/Inputs/source-interleave.ll > %t3.ll
+# RUN: sed -e "s,SRC_COMPDIR,..,g" %p/Inputs/source-interleave.ll > %t4.ll
+
+# RUN: llc -o %t/a/a.o -filetype=obj -mtriple=x86_64-pc-linux %t.ll
+# RUN: llc -o %t/a/b.o -filetype=obj -mtriple=x86_64-pc-linux %t2.ll
+# RUN: llc -o %t/a/c.o -filetype=obj -mtriple=x86_64-pc-linux %t3.ll
+# RUN: llc -o %t/a/d.o -filetype=obj -mtriple=x86_64-pc-linux %t4.ll
+
+# RUN: cd %t
+# RUN: llvm-objdump --source a/a.o | FileCheck %s --check-prefixes=CHECK,SOURCE
+# RUN: llvm-objdump --source a/b.o | FileCheck %s --implicit-check-not=main
+# RUN: llvm-objdump --source a/c.o | FileCheck %s --implicit-check-not=main
+# RUN: llvm-objdump --source a/d.o | FileCheck %s --implicit-check-not=main
+# RUN: cd a
+# RUN: llvm-objdump --source a.o | FileCheck %s --implicit-check-not=main
+# RUN: llvm-objdump --source b.o | FileCheck %s --check-prefixes=CHECK,SOURCE
+# RUN: llvm-objdump --source c.o | FileCheck %s --check-prefixes=CHECK,SOURCE
+# RUN: llvm-objdump --source d.o | FileCheck %s --implicit-check-not=main
+# RUN: cd b
+# RUN: llvm-objdump --source ../a.o | FileCheck %s --implicit-check-not=main
+# RUN: llvm-objdump --source ../b.o | FileCheck %s --implicit-check-not=main
+# RUN: llvm-objdump --source ../c.o | FileCheck %s --implicit-check-not=main
+# RUN: llvm-objdump --source ../d.o | FileCheck %s --check-prefixes=CHECK,SOURCE
+
+# CHECK:       0000000000000010 main:
+# SOURCE-NEXT: ; int main() {
+# CHECK-NEXT:   10:   55                      pushq   %rbp

llvm/trunk/test/tools/llvm-objdump/X86/source-interleave-x86_64.ll

-;  RUN: sed -e "s,SRC_COMPDIR,%/p/Inputs,g" %s > %t.ll
-;  RUN: llc  -o %t.o -filetype=obj -mtriple=x86_64-pc-linux  %t.ll
-;  RUN: llvm-objdump -d -l %t.o >%t0
-;  RUN: llvm-objdump -dl %t.o >%t1
-;  RUN: llvm-objdump -d -S %t.o >%t2
-;  RUN: llvm-objdump -dS %t.o >%t3
-;  RUN: cmp %t0 %t1
-;  RUN: cmp %t2 %t3
-;  RUN: FileCheck --check-prefix=LINES %t.ll < %t0
-;  RUN: FileCheck --check-prefix=SOURCE --strict-whitespace %t.ll < %t2
-; ModuleID = 'source-interleave-x86_64.bc'
-source_filename = "source-interleave-x86_64.c"
-target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-unknown-linux-gnu"
-
-@a = global i32 1, align 4, !dbg !4
-
-; Function Attrs: nounwind uwtable
-define i32 @foo() #0 !dbg !9 {
-entry:
-  %0 = load i32, i32* @a, align 4, !dbg !12
-  ret i32 %0, !dbg !13
-}
-
-; Function Attrs: nounwind uwtable
-define i32 @main() #0 !dbg !14 {
-entry:
-  %retval = alloca i32, align 4
-  %b = alloca i32*, align 8
-  store i32 0, i32* %retval, align 4
-  call void @llvm.dbg.declare(metadata i32** %b, metadata !15, metadata !17), !dbg !18
-  store i32* @a, i32** %b, align 8, !dbg !18
-  %0 = load i32*, i32** %b, align 8, !dbg !19
-  %1 = load i32, i32* %0, align 4, !dbg !20
-  %call = call i32 @foo(), !dbg !21
-  %add = add nsw i32 %1, %call, !dbg !22
-  ret i32 %add, !dbg !23
-}
-
-; Function Attrs: nounwind readnone
-declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
-
-attributes #0 = { nounwind uwtable "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
-attributes #1 = { nounwind readnone }
-
-!llvm.dbg.cu = !{!0}
-!llvm.module.flags = !{!6, !7}
-!llvm.ident = !{!8}
-
-!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 4.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, globals: !3)
-!1 = !DIFile(filename: "source-interleave-x86_64.c", directory: "SRC_COMPDIR")
-!2 = !{}
-!3 = !{!4}
-!4 = !DIGlobalVariableExpression(var: !DIGlobalVariable(name: "a", scope: !0, file: !1, line: 1, type: !5, isLocal: false, isDefinition: true), expr: !DIExpression())
-!5 = !DIBasicType(name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
-!6 = !{i32 2, !"Dwarf Version", i32 4}
-!7 = !{i32 2, !"Debug Info Version", i32 3}
-!8 = !{!"clang version 4.0.0"}
-!9 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 2, type: !10, isLocal: false, isDefinition: true, scopeLine: 2, isOptimized: false, unit: !0, retainedNodes: !2)
-!10 = !DISubroutineType(types: !11)
-!11 = !{!5}
-!12 = !DILocation(line: 3, column: 10, scope: !9)
-!13 = !DILocation(line: 3, column: 3, scope: !9)
-!14 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 6, type: !10, isLocal: false, isDefinition: true, scopeLine: 6, isOptimized: false, unit: !0, retainedNodes: !2)
-!15 = !DILocalVariable(name: "b", scope: !14, file: !1, line: 7, type: !16)
-!16 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !5, size: 64, align: 64)
-!17 = !DIExpression()
-!18 = !DILocation(line: 7, column: 8, scope: !14)
-!19 = !DILocation(line: 8, column: 11, scope: !14)
-!20 = !DILocation(line: 8, column: 10, scope: !14)
-!21 = !DILocation(line: 8, column: 15, scope: !14)
-!22 = !DILocation(line: 8, column: 13, scope: !14)
-!23 = !DILocation(line: 8, column: 3, scope: !14)
-; LINES: main:
-; LINES-NEXT: ; {{[ -\(\)_A-Za-z0-9.\\/:]+}}source-interleave-x86_64.c:6
-
-; SOURCE: main:
-; SOURCE-NEXT: ; int main() {
-; SOURCE:      ;   int *b = &a;

llvm/trunk/test/tools/llvm-objdump/X86/source-interleave-x86_64.test

+# RUN: sed -e "s,SRC_COMPDIR,%/p/Inputs,g" %p/Inputs/source-interleave.ll > %t.ll
+# RUN: llc -o %t.o -filetype=obj -mtriple=x86_64-pc-linux %t.ll
+# RUN: llvm-objdump -d -l %t.o >%t0
+# RUN: llvm-objdump -dl %t.o >%t1
+# RUN: llvm-objdump -d -S %t.o >%t2
+# RUN: llvm-objdump -dS %t.o >%t3
+# RUN: cmp %t0 %t1
+# RUN: cmp %t2 %t3
+# RUN: FileCheck --check-prefix=LINES %s < %t0
+# RUN: FileCheck --check-prefix=SOURCE --strict-whitespace %s < %t2
+
+# LINES: main:
+# LINES-NEXT: ; {{[ -\(\)_A-Za-z0-9.\\/:]+}}source-interleave-x86_64.c:6
+
+# SOURCE: main:
+# SOURCE-NEXT: ; int main() {
+# SOURCE:      ;   int *b = &a;