labath created this revision.
labath added reviewers: vsk, djtodoro, dblaikie.
Herald added a project: LLDB.

This function rewrites the test to be (hopefully) less susceptible to
codegen changes and re-enables it.

The most interesting changes are:

- use an __attribute__((optnone)) function instead of a volatile asm to "use" a 
value. This isn't strictly necessary, but it makes the function simpler while 
achieving the same effect.
- use a call to a function with the exact same signature instead of a volatile 
asm to "destroy" arguments. This makes the independent of the ABI, and 
(together with avoiding the usage of the arguments after the call) ensures that 
the compiler has no reason to move the argument from its initial register 
(previously we needed to guess where will the compiler store the arguments).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79491

Files:
  
lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py
  
lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp

Index: lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp
===================================================================
--- lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp
+++ lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/main.cpp
@@ -1,105 +1,61 @@
-// Note: This test requires the SysV AMD64 ABI to be in use, and requires
-// compiler support for DWARF entry values.
-
-// Inhibit dead-arg-elim by using 'x'.
-template<typename T> __attribute__((noinline)) void use(T x) {
-  asm volatile (""
-      /* Outputs */  :
-      /* Inputs */   : "g"(x)
-      /* Clobbers */ :
-  );
-}
+// Note: This test requires compiler support for DWARF entry values.
+
+int dummy;
+volatile int global = 0;
 
-// Destroy %rsi in the current frame.
-#define DESTROY_RSI \
-  asm volatile ("xorq %%rsi, %%rsi" \
-      /* Outputs */  : \
-      /* Inputs */   : \
-      /* Clobbers */ : "rsi" \
-  );
-
-// Destroy %rbx in the current frame.
-#define DESTROY_RBX \
-  asm volatile ("xorq %%rbx, %%rbx" \
-      /* Outputs */  : \
-      /* Inputs */   : \
-      /* Clobbers */ : "rbx" \
-  );
+template <typename... T> __attribute__((optnone)) void use(T...) {}
 
 struct S1 {
   int field1 = 123;
   int *field2 = &field1;
 };
 
-__attribute__((noinline))
-void func1(int &sink, int x) {
-  use(x);
-
-  // Destroy 'x' in the current frame.
-  DESTROY_RSI;
-
-  // NOTE: Currently, we do not generate DW_OP_entry_value for the 'x',
-  // since it gets copied into a register that is not callee saved,
-  // and we can not guarantee that its value has not changed.
-
-  ++sink;
-
-  // Destroy 'sink' in the current frame.
-  DESTROY_RBX;
+__attribute__((noinline)) void func1(int &sink) {
+  // First use works around a compiler "bug" where an unused variable gets
+  // no location descriptions. The second use overwrites the function arguments
+  // with other values.
+  use<int &>(sink);
+  use<int &>(dummy);
 
+  ++global;
   //% self.filecheck("image lookup -va $pc", "main.cpp", "-check-prefix=FUNC1-DESC")
   // FUNC1-DESC: name = "sink", type = "int &", location = DW_OP_entry_value(DW_OP_reg5 RDI)
 }
 
 __attribute__((noinline))
 void func2(int &sink, int x) {
-  use(x);
-
-  // Destroy 'x' in the current frame.
-  DESTROY_RSI;
-
-  //% self.filecheck("expr x", "main.cpp", "-check-prefix=FUNC2-EXPR-FAIL", expect_cmd_failure=True)
-  // FUNC2-EXPR-FAIL: couldn't get the value of variable x: variable not available
-
-  ++sink;
-
-  // Destroy 'sink' in the current frame.
-  DESTROY_RBX;
-
-  //% self.filecheck("expr sink", "main.cpp", "-check-prefix=FUNC2-EXPR")
-  // FUNC2-EXPR: ${{.*}} = 2
+  use<int &, int>(sink, x);
+  use<int &, int>(dummy, 0);
+
+  ++global;
+  //% self.filecheck("expr x", "main.cpp", "-check-prefix=FUNC2-EXPR1")
+  //% self.filecheck("expr sink", "main.cpp", "-check-prefix=FUNC2-EXPR2")
+  // FUNC2-EXPR1: ${{.*}} = 123
+  // FUNC2-EXPR2: ${{.*}} = 2
 }
 
 __attribute__((noinline))
 void func3(int &sink, int *p) {
-  use(p);
-
-  // Destroy 'p' in the current frame.
-  DESTROY_RSI;
+  use<int &, int *>(sink, p);
+  use<int &, int *>(dummy, nullptr);
 
   //% self.filecheck("expr *p", "main.cpp", "-check-prefix=FUNC3-EXPR")
   // FUNC3-EXPR: (int) ${{.*}} = 123
-
-  ++sink;
 }
 
 __attribute__((noinline))
 void func4_amb(int &sink, int x) {
-  use(x);
-
-  // Destroy 'x' in the current frame.
-  DESTROY_RSI;
-
-  //% self.filecheck("expr x", "main.cpp", "-check-prefix=FUNC4-EXPR-FAIL", expect_cmd_failure=True)
-  // FUNC4-EXPR-FAIL: couldn't get the value of variable x: variable not available
-
-  ++sink;
-
-  // Destroy 'sink' in the current frame.
-  DESTROY_RBX;
-
-  //% self.filecheck("expr sink", "main.cpp", "-check-prefix=FUNC4-EXPR", expect_cmd_failure=True)
-  // FUNC4-EXPR: couldn't get the value of variable sink: Could not evaluate DW_OP_entry_value.
+  use<int &, int>(sink, x);
+  use<int &, int>(dummy, 0);
+
+  ++global;
+  //% self.filecheck("expr x", "main.cpp", "-check-prefix=FUNC4-EXPR-FAIL",
+  //%     expect_cmd_failure=True)
+  //% self.filecheck("expr sink", "main.cpp","-check-prefix=FUNC4-EXPR",
+  //%     expect_cmd_failure=True)
+  // FUNC4-EXPR-FAIL: couldn't get the value of variable x: Could not evaluate DW_OP_entry_value.
+  // FUNC4-EXPR: couldn't get the value of variable sink:
+  // Could not evaluate DW_OP_entry_value.
 }
 
 __attribute__((noinline))
@@ -120,21 +76,14 @@
   // FUNC7-BT-NEXT: [inlined] func8_inlined
   // FUNC7-BT-NEXT: [inlined] func9_inlined
   // FUNC7-BT-NEXT: func10
-  use(x);
-
-  // Destroy 'x' in the current frame.
-  DESTROY_RSI;
-
-  //% self.filecheck("expr x", "main.cpp", "-check-prefix=FUNC7-EXPR-FAIL", expect_cmd_failure=True)
-  // FUNC7-EXPR-FAIL: couldn't get the value of variable x: variable not available
-
-  ++sink;
-
-  // Destroy 'sink' in the current frame.
-  DESTROY_RBX;
-
-  //% self.filecheck("expr sink", "main.cpp", "-check-prefix=FUNC7-EXPR")
-  // FUNC7-EXPR: ${{.*}} = 4
+  use<int &, int>(sink, x);
+  use<int &, int>(dummy, 0);
+
+  ++global;
+  //% self.filecheck("expr x", "main.cpp", "-check-prefix=FUNC7-EXPR1")
+  //% self.filecheck("expr sink", "main.cpp", "-check-prefix=FUNC7-EXPR2")
+  // FUNC7-EXPR1: ${{.*}} = 123
+  // FUNC7-EXPR2: ${{.*}} = 5
 }
 
 __attribute__((always_inline))
@@ -157,21 +106,14 @@
   //% self.filecheck("bt", "main.cpp", "-check-prefix=FUNC11-BT")
   // FUNC11-BT: func11_tailcalled{{.*}}
   // FUNC11-BT-NEXT: func12{{.*}} [artificial]
-  use(x);
-
-  // Destroy 'x' in the current frame.
-  DESTROY_RSI;
-
-  //% self.filecheck("expr x", "main.cpp", "-check-prefix=FUNC11-EXPR-FAIL", expect_cmd_failure=True)
-  // FUNC11-EXPR-FAIL: couldn't get the value of variable x: variable not available
-
-  ++sink;
-
-  // Destroy 'sink' in the current frame.
-  DESTROY_RBX;
-
-  //% self.filecheck("expr sink", "main.cpp", "-check-prefix=FUNC11-EXPR")
-  // FUNC11-EXPR: ${{.*}} = 5
+  use<int &, int>(sink, x);
+  use<int &, int>(dummy, 0);
+
+  ++global;
+  //% self.filecheck("expr x", "main.cpp", "-check-prefix=FUNC11-EXPR1")
+  //% self.filecheck("expr sink", "main.cpp", "-check-prefix=FUNC11-EXPR2")
+  // FUNC11-EXPR1: ${{.*}} = 123
+  // FUNC11-EXPR2: ${{.*}} = 5
 }
 
 __attribute__((noinline))
@@ -184,21 +126,15 @@
   //% self.filecheck("bt", "main.cpp", "-check-prefix=FUNC13-BT")
   // FUNC13-BT: func13{{.*}}
   // FUNC13-BT-NEXT: func14{{.*}}
-  use(x);
-
-  // Destroy 'x' in the current frame.
-  DESTROY_RSI;
-
-  //% self.filecheck("expr x", "main.cpp", "-check-prefix=FUNC13-EXPR-FAIL", expect_cmd_failure=True)
-  // FUNC13-EXPR-FAIL: couldn't get the value of variable x: variable not available
-
-  use(sink);
+  use<int &, int>(sink, x);
+  use<int &, int>(dummy, 0);
 
-  // Destroy 'sink' in the current frame.
-  DESTROY_RBX;
+  ++global;
 
-  //% self.filecheck("expr sink", "main.cpp", "-check-prefix=FUNC13-EXPR")
-  // FUNC13-EXPR: ${{.*}} = 5
+  //% self.filecheck("expr x", "main.cpp", "-check-prefix=FUNC13-EXPR1")
+  //% self.filecheck("expr sink", "main.cpp", "-check-prefix=FUNC13-EXPR2")
+  // FUNC13-EXPR1: ${{.*}} = 123
+  // FUNC13-EXPR2: ${{.*}} = 5
 }
 
 __attribute__((noinline, disable_tail_calls))
@@ -219,8 +155,9 @@
   S1 s1;
 
   // Test location dumping for DW_OP_entry_value.
-  func1(sink, 123);
+  func1(sink);
 
+  sink = 2;
   // Test evaluation of "DW_OP_constu" in the parent frame.
   func2(sink, 123);
 
@@ -235,6 +172,7 @@
   // evaluation should fail.
   func6(sink, 123);
 
+  sink = 5;
   // Test that evaluation can "see through" inlining.
   func10(sink, 123);
 
Index: lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py
===================================================================
--- lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py
+++ lldb/test/API/functionalities/param_entry_vals/basic_entry_values_x86_64/TestBasicEntryValuesX86_64.py
@@ -6,8 +6,7 @@
 supported_platforms.extend(lldbplatformutil.getDarwinOSTriples())
 
 lldbinline.MakeInlineTest(__file__, globals(),
-        [decorators.skipIf(bugnumber="llvm.org/PR44774"),
-         decorators.skipUnlessPlatform(supported_platforms),
+        [decorators.skipUnlessPlatform(supported_platforms),
          decorators.skipIf(compiler="clang", compiler_version=['<', '10.0']),
          decorators.skipUnlessArch('x86_64'),
          decorators.skipUnlessHasCallSiteInfo,
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to