Test data for symbol lookup. NFC

[sepavloff]

Summary

This change adds ELF file with different kinds of symbols to be used in
test for symbol lookup. It is a prerequisite for implementation of
symbol lookup in llvm-symbolizer.

[ikudrin]

This patch only adds files to be used in another patch. It does not change any code or add any new tests and has no value of its own. It should be merged with the main patch, or relevant parts of D149759 should be moved here, whichever is preferable.

llvm/test/tools/llvm-symbolizer/Inputs/symbols.part3.c

13

Please remove the blank line at EOF

llvm/test/tools/llvm-symbolizer/Inputs/symbols.part4.c

14

Likewise

[MaskRay]

I wonder whether these files should go to cross-project-tests/

[jhenderson]

In D149757#4630137, @MaskRay wrote:

I wonder whether these files should go to cross-project-tests/

One of my original motivations for making cross-project-tests included finding a good way to runtime generate llvm-symbolizer test inputs (see https://lists.llvm.org/pipermail/llvm-dev/2021-January/148048.html), rather than relying on canned binaries, so I agree that it's likely a good idea to move those sorts of tests.

[dblaikie]

In D149757#4630300, @jhenderson wrote:

In D149757#4630137, @MaskRay wrote:

I wonder whether these files should go to cross-project-tests/

One of my original motivations for making cross-project-tests included finding a good way to runtime generate llvm-symbolizer test inputs (see https://lists.llvm.org/pipermail/llvm-dev/2021-January/148048.html), rather than relying on canned binaries, so I agree that it's likely a good idea to move those sorts of tests.

Conversely, all features should be tested in the subproject - cross-project-tests can be for broader integration testing, but it shouldn't mean we see code changes in subprojects without tests in those subprojects. If these tests are the "wider" sort of testing - testing interactions between different parts of the code, where the parts are tested each in isolation in the subproject, then that seems OK to me to move these wider tests out to cross-project-tests.

(maybe that's a formalization of the heuristic: All code should be covered, ideally all conditions should be falsifiable (eg: if you add a ! in some condition, a test should fail - though I understand this is a fairly high/lofty/not-often-achieved goal) by in-subproject tests, but checking that codepath A1 interacts well with codepath B2 may be more suitably done in cross-project-tests, for instance)

[sepavloff]

Change the patch according to review notes

[sepavloff]

I don't think building test binaries in test time is possible for this particular case. The tests using the binary from here expect symbols at concrete locations. If something in code generation changes, and the locations become different, the tests would fail.

In D149757#4549272, @ikudrin wrote:

This patch only adds files to be used in another patch. It does not change any code or add any new tests and has no value of its own. It should be merged with the main patch, or relevant parts of D149759 should be moved here, whichever is preferable.

You are right, these changes are a part of D149759. They are however independent of it and moving them to a separate patch could facilitate the review process.

[dblaikie]

In D149757#4635969, @sepavloff wrote:

I don't think building test binaries in test time is possible for this particular case. The tests using the binary from here expect symbols at concrete locations. If something in code generation changes, and the locations become different, the tests would fail.

In D149757#4549272, @ikudrin wrote:

This patch only adds files to be used in another patch. It does not change any code or add any new tests and has no value of its own. It should be merged with the main patch, or relevant parts of D149759 should be moved here, whichever is preferable.

You are right, these changes are a part of D149759. They are however independent of it and moving them to a separate patch could facilitate the review process.

Precommitting tests is encouraged by some (I'm not a huge fan, but I appreciate the benefits) - the purpose is to precommit a running test that demonstrates the existing (problematic/broken/buggy/suboptimal) behavior, then when the fix is committed, it includes only the changes to the test that demonstrate the improvement/fix - this makes it easier to see the change the test causes (rather than seeing the whole test and being unsure about what part of what the test is testing is new/different behavior)

Committing the files without executing them as a test doesn't provide this value and shouldn't be done - then the files are committed without any verification of their behavior, obscuring their intent in this commit and in the subsequent bug fix.

Please either include a running test that verifies the buggy behavior in a precommit like this, or roll these files into the fix commit.

If @jhenderson would prefer them separate for review, but combined for commit, that's OK/I'll leave it up to him.

[jhenderson]

Strong +1 to what both @ikudrin and @dblaikie have said: these files on their own don't do anything useful (not even add a test), so they shouldn't be added on their own in a separate commit/review. Please merge them into the review that uses them, or create the test that uses them, but showing the old behaviour instead (I think the former is my preference in this context).

I think I have concerns about these files in their current state anyway, but I'd like to avoid commenting on them until they are in the "right" patch.

[sepavloff]

It has been merged to D149759.

llvm/test/tools/llvm-symbolizer/Inputs/symbols.h

+// This file is a part of sources used to build `symbols.so`, which is used to
+// test symbol location search made by llvm-symbolizer.
+//
+// Build instructions:
+// $ mkdir /tmp/dbginfo
+// $ cp symbols.h symbols.part1.cpp symbols.part2.cpp symbols.part3.c symbols.part4.c /tmp/dbginfo/
+// $ cd /tmp/dbginfo
+// $ gcc -osymbols.so -shared -fPIC -g symbols.part1.cpp symbols.part2.cpp symbols.part3.c symbols.part4.c
+
+
+extern "C" {
+extern int global_01;
+int func_01();
+int func_02(int);
+}
+
+template<typename T> T func_03(T x) {
+  return x + T(1);
+}

llvm/test/tools/llvm-symbolizer/Inputs/symbols.part1.cpp

+#include "symbols.h"
+
+int global_01 = 22;
+
+int static static_var = 0;
+
+static int static_func_01(int x) {
+  static_var = x;
+  return global_01;
+}
+
+int func_01() {
+  int res = 1;
+  return res + static_func_01(22);
+}
+
+int func_04() {
+  static_var = 0;
+  return 22;
+}
+
+int func_04(int x) {
+  int res = static_var;
+  return res + func_03(x);
+}

llvm/test/tools/llvm-symbolizer/Inputs/symbols.part2.cpp

+#include "symbols.h"
+
+int static static_var = 4;
+
+static int static_func_01(int x) {
+  static_var--;
+  return x;
+}
+
+int func_02(int x) {
+  static_var = x;
+  return static_func_01(x);
+}
+
+int func_05(int x) {
+  int res = static_var;
+  return res + func_03(x);
+}

llvm/test/tools/llvm-symbolizer/Inputs/symbols.part3.c

+static int static_func(int);
+static int static_var = 0;
+
+int static_func(int x) {
+  static_var++;
+  return static_var + x;
+}
+
+int func_06(int x) {
+  return static_func(x);
+}

llvm/test/tools/llvm-symbolizer/Inputs/symbols.part4.c

+static int static_func(int);
+static int static_var = 5;
+
+int static_func(int x) {
+  static_var++;
+  return static_var + x;
+}
+
+int func_07(int x) {
+  static_var++;
+  return static_func(x);
+}