[committed] Use TEST_EXECS_$(UNAME) in Makefile

[committed] Use TEST_EXECS_$(UNAME) in Makefile

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 874 bytes --]

[ was: Re: [committed] Only compile test-cases using hardcoded assembly
for x86_64 ]

On 12-12-2019 10:20, Jakub Jelinek wrote:
> On Thu, Dec 12, 2019 at 10:07:16AM +0100, Tom de Vries wrote:
>> 2019-12-12  Tom de Vries  <tdevries@suse.de>
>>
>> 	* Makefile (UNAME): New variable.
>> 	(dw2-restrict, dw2-skip-prologue, py-section-script)
>> 	(implptr-64bit-d2o4a8r8t0, varval): Only compile for x86_64.
> 
> I meant something shorter/easier to use, like:
> TEST_EXECS_x86_64 = dw2-restrict py-section-script dw2-skip-prologue \
> 	implptr-64bit-d2o4a8r8t0 varval
> TEST_EXECS = hello dwz-for-test min two-typedef start hello-gold-gdb-index \
>         start-gold hello-gnu-pubnames $(TEST_EXECS_DWARF_ASM) \
> 	$(TEST_EXECS_$(UNAME))
> which would be easily extensible for tests specific to other architectures.

Ack, committed patch that uses that approach.

Thanks,
- Tom

[-- Attachment #2: 0001-Use-TEST_EXECS_-UNAME-in-Makefile.patch --]
[-- Type: text/x-patch, Size: 3555 bytes --]

Use TEST_EXECS_$(UNAME) in Makefile

Currently we test for $(UNAME) == x86_64 in some make targets.

Instead, filter them out in TEST_EXECS, by means of moving them to
TEST_EXECS_x86_64 and including them conditionally using
$(TEST_EXECS_$(UNAME)).

Also tested with UNAME setting commented out in Makefile to test behaviour on
non-x86_64 architectures.

2019-12-12  Tom de Vries  <tdevries@suse.de>

	* Makefile (TEST_EXECS_x86_64): New variable, factored out of ...
	(TEST_EXECS): ... here.  Include TEST_EXECS_x86_64 conditionally using
	$(TEST_EXECS_$(UNAME)).
	* testsuite/dwz.tests/dwz-tests.exp: Handle missing required
	executables. Add missing break in foreach.

---
 Makefile                          | 29 +++++++++++------------------
 testsuite/dwz.tests/dwz-tests.exp |  6 ++++++
 2 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/Makefile b/Makefile
index 34c4b47..770824b 100644
--- a/Makefile
+++ b/Makefile
@@ -27,9 +27,11 @@ PWD:=$(shell pwd -P)
 
 TEST_SRC = $(srcdir)/testsuite/dwz.tests
 TEST_EXECS_DWARF_ASM = no-multifile-prop invalid-dw-at-stmt-list-encoding
-TEST_EXECS = hello py-section-script dwz-for-test min two-typedef \
-	dw2-skip-prologue start implptr-64bit-d2o4a8r8t0 hello-gold-gdb-index \
-	start-gold hello-gnu-pubnames varval $(TEST_EXECS_DWARF_ASM)
+TEST_EXECS_x86_64 = py-section-script dw2-skip-prologue \
+	implptr-64bit-d2o4a8r8t0 varval
+TEST_EXECS = hello dwz-for-test min two-typedef start hello-gold-gdb-index \
+	start-gold hello-gnu-pubnames $(TEST_EXECS_DWARF_ASM) \
+	$(TEST_EXECS_$(UNAME))
 
 UNAME:=$(shell uname -p)
 
@@ -40,16 +42,11 @@ hello-gnu-pubnames:
 	$(CC) $(TEST_SRC)/hello.c -o $@ -g -ggnu-pubnames || touch $@
 
 dw2-skip-prologue:
-	if [ $(UNAME) = "x86_64" ]; then \
-	  $(CC) $(TEST_SRC)/dw2-skip-prologue.S \
-	    $(TEST_SRC)/dw2-skip-prologue.c \
-	    -DINLINED -DPTRBITS=64 -o $@; \
-	else touch $@; fi
+	$(CC) $(TEST_SRC)/dw2-skip-prologue.S $(TEST_SRC)/dw2-skip-prologue.c \
+	  -DINLINED -DPTRBITS=64 -o $@
 
 py-section-script:
-	if [ $(UNAME) = "x86_64" ]; then \
-	  $(CC) $(TEST_SRC)/py-section-script.s -o $@ -g ; \
-	else touch $@; fi
+	$(CC) $(TEST_SRC)/py-section-script.s -o $@ -g
 
 DWZ_TEST_SOURCES := $(patsubst %.o,%-for-test.c,$(OBJECTS))
 
@@ -75,19 +72,15 @@ start-gold:
 	$(CC) $(TEST_SRC)/start.c -fuse-ld=gold -o $@ -g -nostdlib || touch $@
 
 implptr-64bit-d2o4a8r8t0:
-	if [ $(UNAME) = "x86_64" ]; then \
-	  $(CC) $(TEST_SRC)/implptr-64bit-d2o4a8r8t0.S $(TEST_SRC)/main.c \
-	  -o $@ -g; \
-	else touch $@; fi
+	$(CC) $(TEST_SRC)/implptr-64bit-d2o4a8r8t0.S $(TEST_SRC)/main.c \
+	  -o $@ -g
 
 hello-gold-gdb-index:
 	$(CC) $(TEST_SRC)/hello.c -g -fuse-ld=gold -Wl,--gdb-index -o $@ \
 	    || touch $@
 
 varval:
-	if [ $(UNAME) = "x86_64" ]; then \
-	  $(CC) $(TEST_SRC)/varval.c $(TEST_SRC)/varval.S -g -o $@; \
-	else touch $@; fi
+	$(CC) $(TEST_SRC)/varval.c $(TEST_SRC)/varval.S -g -o $@
 
 POINTER_SIZE:=$(shell $(CC) $(TEST_SRC)/pointer-size.c -o pointer-size; \
 	./pointer-size; \
diff --git a/testsuite/dwz.tests/dwz-tests.exp b/testsuite/dwz.tests/dwz-tests.exp
index a7e2dee..2e5d19a 100644
--- a/testsuite/dwz.tests/dwz-tests.exp
+++ b/testsuite/dwz.tests/dwz-tests.exp
@@ -84,9 +84,15 @@ foreach test $tests {
 
     set unsupported 0
     foreach required_exec $required_execs {
+	set exists [file exists $required_exec]
+	if { $exists == 0 } {
+	    set unsupported 1
+	    break
+	}
 	set size [file size $required_exec]
 	if { $size == 0 } {
 	    set unsupported 1
+	    break
 	}
     }
     if { $unsupported == 1 } {

Re: [committed] Only compile test-cases using hardcoded assembly for x86_64

[Jakub Jelinek]

On Thu, Dec 12, 2019 at 10:07:16AM +0100, Tom de Vries wrote:
> 2019-12-12  Tom de Vries  <tdevries@suse.de>
> 
> 	* Makefile (UNAME): New variable.
> 	(dw2-restrict, dw2-skip-prologue, py-section-script)
> 	(implptr-64bit-d2o4a8r8t0, varval): Only compile for x86_64.

I meant something shorter/easier to use, like:
TEST_EXECS_x86_64 = dw2-restrict py-section-script dw2-skip-prologue \
	implptr-64bit-d2o4a8r8t0 varval
TEST_EXECS = hello dwz-for-test min two-typedef start hello-gold-gdb-index \
        start-gold hello-gnu-pubnames $(TEST_EXECS_DWARF_ASM) \
	$(TEST_EXECS_$(UNAME))
which would be easily extensible for tests specific to other architectures.

	Jakub

Re: x86_64 specific tests in dwz vs. other architectures

[Jakub Jelinek]

On Wed, Dec 11, 2019 at 10:33:33PM +0100, Tom de Vries wrote:
> > /builddir/build/BUILD/dwz/testsuite/dwz.tests/dw2-skip-prologue.S:309: Error: cannot represent relocation type BFD_RELOC_64
> > as: /tmp/ccg3sw27.o: unsupported relocation type: 0x1
> > on i686 etc.  Can't we ensure that the tests are assembled only on the
> > corresponding architecture and nowhere else if they are architecture
> > specific?
> > It is fine if some tests are UNSUPPORTED, just we should completely skip
> > them if they can't be assembled.
> 
> Indeed, the messages are ugly, but it's not a correctness problem, just
> an annoyance.
> 
> The test execs are build directly from the makefile, not by dejagnu, so
> we can't use the target testing available there. I suppose I could test
> for uname -p:
> ...
> $ uname -p
> x86_64
> ...

Yeah, and if we e.g. require GNU make to build dwz, we could use all kinds
of GNU make features to conditionalize it easily.
Though, at least for the tests where gdb isn't run on it, perhaps it might
be good enough to replace actual instructions in the assembly sources with
.skip or similar; of course it depends also if the location info doesn't
contain something horribly wrong on other architecture, probably can't mix
32-bit and 64-bit and some arches use different characters in assembly, so
maybe some portability macros and preprocessing would be needed (e.g.
arm and a few others require %progbits rather than @progbits).

	Jakub

Re: x86_64 specific tests in dwz vs. other architectures

[Tom de Vries]

> Hi!
> 
> I've only got now to try building dwz 0.13 (just a scratch build:
> https://koji.fedoraproject.org/koji/taskinfo?taskID=39485926
> ) in Fedora,

Just FYI, there's a dwz-0.13-branch that I've maintained that is 0.13 +
fixes.

> but looking at the build files, I see on non-x86_64
> arches ugly messages like:
> dw2-restrict.c: Assembler messages:
> dw2-restrict.c:55: Error: unrecognized opcode: `movq'
> dw2-restrict.c:58: Error: unrecognized opcode: `movq'
> dw2-restrict.c:59: Error: unrecognized opcode: `movsbl'
> dw2-restrict.c:60: Error: unrecognized opcode: `ret'
> dw2-restrict.c:76: Error: unrecognized opcode: `pushq'
> dw2-restrict.c:78: Error: unrecognized opcode: `movq'
> dw2-restrict.c:80: Error: unrecognized opcode: `subq'
> dw2-restrict.c:82: Error: unrecognized opcode: `leaq'
> dw2-restrict.c:83: Error: unrecognized opcode: `movl'
> dw2-restrict.c:86: Error: unrecognized opcode: `callq'
> dw2-restrict.c:87: Error: unrecognized opcode: `addq'
> dw2-restrict.c:88: Error: unrecognized opcode: `popq'
> dw2-restrict.c:89: Error: unrecognized opcode: `ret'
> on ppc64le, 
> dw2-restrict.c: Assembler messages:
> dw2-restrict.c:55: Error: bad register name `%rdi'
> dw2-restrict.c:58: Error: bad register name `%rsp)'
> dw2-restrict.c:59: Error: bad register name `%rdi)'
> dw2-restrict.c:76: Error: bad register name `%rbp'
> dw2-restrict.c:78: Error: bad register name `%rsp'
> dw2-restrict.c:80: Error: bad register name `%rsp'
> dw2-restrict.c:82: Error: bad register name `%rdi'
> dw2-restrict.c:83: Error: bad register name `%rbp)'
> dw2-restrict.c:86: Error: invalid instruction suffix for `call'
> dw2-restrict.c:87: Error: bad register name `%rsp'
> dw2-restrict.c:88: Error: bad register name `%rbp'
> ...
> /builddir/build/BUILD/dwz/testsuite/dwz.tests/dw2-skip-prologue.S:309: Error: cannot represent relocation type BFD_RELOC_64
> as: /tmp/ccg3sw27.o: unsupported relocation type: 0x1
> on i686 etc.  Can't we ensure that the tests are assembled only on the
> corresponding architecture and nowhere else if they are architecture
> specific?
> It is fine if some tests are UNSUPPORTED, just we should completely skip
> them if they can't be assembled.

Indeed, the messages are ugly, but it's not a correctness problem, just
an annoyance.

The test execs are build directly from the makefile, not by dejagnu, so
we can't use the target testing available there. I suppose I could test
for uname -p:
...
$ uname -p
x86_64
...

Thanks,
- Tom

[committed] Only compile test-cases using hardcoded assembly for x86_64

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 2320 bytes --]

[ was: Re: x86_64 specific tests in dwz vs. other architectures ]

On 11-12-2019 23:12, Jakub Jelinek wrote:
> On Wed, Dec 11, 2019 at 10:33:33PM +0100, Tom de Vries wrote:
>>> /builddir/build/BUILD/dwz/testsuite/dwz.tests/dw2-skip-prologue.S:309: Error: cannot represent relocation type BFD_RELOC_64
>>> as: /tmp/ccg3sw27.o: unsupported relocation type: 0x1
>>> on i686 etc.  Can't we ensure that the tests are assembled only on the
>>> corresponding architecture and nowhere else if they are architecture
>>> specific?
>>> It is fine if some tests are UNSUPPORTED, just we should completely skip
>>> them if they can't be assembled.
>>
>> Indeed, the messages are ugly, but it's not a correctness problem, just
>> an annoyance.
>>
>> The test execs are build directly from the makefile, not by dejagnu, so
>> we can't use the target testing available there. I suppose I could test
>> for uname -p:
>> ...
>> $ uname -p
>> x86_64
>> ...
> 
> Yeah, and if we e.g. require GNU make to build dwz,

I thought we already required that, given the use of '+=', which is
listed here (
https://www.gnu.org/software/make/manual/html_node/Features.html ) as
something to avoid for portable Makefiles.

Anyway, we're now using the addsuffix function, which I think is a
GNUism. Also, I suspect the use of the shell function is non-portable,
though I didn't find it mentioned at the link above.

> we could use all kinds
> of GNU make features to conditionalize it easily.
> Though, at least for the tests where gdb isn't run on it, perhaps it might
> be good enough to replace actual instructions in the assembly sources with
> .skip or similar; of course it depends also if the location info doesn't
> contain something horribly wrong on other architecture, probably can't mix
> 32-bit and 64-bit and some arches use different characters in assembly, so
> maybe some portability macros and preprocessing would be needed (e.g.
> arm and a few others require %progbits rather than @progbits).

I've fixed things for now in the most restrictive way: only compile
hardcoded assembly test-cases on x86_64, if they were generated on
x86_64, which should fix all those warnings.

I hope to eventually replace all hardcoded assembly as well as
compressed execs with dwarf assembly, which should work for all
architectures.

Thanks,
- Tom

[-- Attachment #2: 0001-Only-compile-test-cases-using-hardcoded-assembly-for-x86_64.patch --]
[-- Type: text/x-patch, Size: 2913 bytes --]

Only compile test-cases using hardcoded assembly for x86_64

Currently the test-cases using hardcoded assembly are tentatively compiled
for all architectures, and a failure to compile is handled as UNSUPPORTED.

Since the hardcoded assembly is generated on x86_64, this approach produces
a lot of warning on other architectures, which make it harder to spot actual
problems in the log.  Also, this approach might mask compilations error on
x86_64.

Fix this by only compiling these test-cases for x86_64, and expecting a valid
result for x86_64.

2019-12-12  Tom de Vries  <tdevries@suse.de>

	* Makefile (UNAME): New variable.
	(dw2-restrict, dw2-skip-prologue, py-section-script)
	(implptr-64bit-d2o4a8r8t0, varval): Only compile for x86_64.

---
 Makefile | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/Makefile b/Makefile
index fc70bfb..8abb22d 100644
--- a/Makefile
+++ b/Makefile
@@ -31,6 +31,8 @@ TEST_EXECS = hello dw2-restrict py-section-script dwz-for-test min two-typedef \
 	dw2-skip-prologue start implptr-64bit-d2o4a8r8t0 hello-gold-gdb-index \
 	start-gold hello-gnu-pubnames varval $(TEST_EXECS_DWARF_ASM)
 
+UNAME:=$(shell uname -p)
+
 hello:
 	$(CC) $(TEST_SRC)/hello.c -o $@ -g
 
@@ -38,14 +40,21 @@ hello-gnu-pubnames:
 	$(CC) $(TEST_SRC)/hello.c -o $@ -g -ggnu-pubnames || touch $@
 
 dw2-restrict:
-	$(CC) -no-pie $(TEST_SRC)/dw2-restrict.S -o $@ || touch $@
+	if [ $(UNAME) = "x86_64" ]; then \
+	  $(CC) -no-pie $(TEST_SRC)/dw2-restrict.S -o $@; \
+	else touch $@; fi
 
 dw2-skip-prologue:
-	$(CC) $(TEST_SRC)/dw2-skip-prologue.S $(TEST_SRC)/dw2-skip-prologue.c \
-	  -DINLINED -DPTRBITS=64 -o $@ || touch $@
+	if [ $(UNAME) = "x86_64" ]; then \
+	  $(CC) $(TEST_SRC)/dw2-skip-prologue.S \
+	    $(TEST_SRC)/dw2-skip-prologue.c \
+	    -DINLINED -DPTRBITS=64 -o $@; \
+	else touch $@; fi
 
 py-section-script:
-	$(CC) $(TEST_SRC)/py-section-script.s -o $@ -g || touch $@
+	if [ $(UNAME) = "x86_64" ]; then \
+	  $(CC) $(TEST_SRC)/py-section-script.s -o $@ -g ; \
+	else touch $@; fi
 
 DWZ_TEST_SOURCES := $(patsubst %.o,%-for-test.c,$(OBJECTS))
 
@@ -71,16 +80,19 @@ start-gold:
 	$(CC) $(TEST_SRC)/start.c -fuse-ld=gold -o $@ -g -nostdlib || touch $@
 
 implptr-64bit-d2o4a8r8t0:
-	$(CC) $(TEST_SRC)/implptr-64bit-d2o4a8r8t0.S $(TEST_SRC)/main.c -o $@ \
-	  -g || touch $@
+	if [ $(UNAME) = "x86_64" ]; then \
+	  $(CC) $(TEST_SRC)/implptr-64bit-d2o4a8r8t0.S $(TEST_SRC)/main.c \
+	  -o $@ -g; \
+	else touch $@; fi
 
 hello-gold-gdb-index:
 	$(CC) $(TEST_SRC)/hello.c -g -fuse-ld=gold -Wl,--gdb-index -o $@ \
 	    || touch $@
 
 varval:
-	$(CC) $(TEST_SRC)/varval.c $(TEST_SRC)/varval.S -g -o $@ \
-	    || touch $@
+	if [ $(UNAME) = "x86_64" ]; then \
+	  $(CC) $(TEST_SRC)/varval.c $(TEST_SRC)/varval.S -g -o $@; \
+	else touch $@; fi
 
 POINTER_SIZE:=$(shell $(CC) $(TEST_SRC)/pointer-size.c -o pointer-size; \
 	./pointer-size; \

x86_64 specific tests in dwz vs. other architectures

[Jakub Jelinek]

Hi!

I've only got now to try building dwz 0.13 (just a scratch build:
https://koji.fedoraproject.org/koji/taskinfo?taskID=39485926
) in Fedora, but looking at the build files, I see on non-x86_64
arches ugly messages like:
dw2-restrict.c: Assembler messages:
dw2-restrict.c:55: Error: unrecognized opcode: `movq'
dw2-restrict.c:58: Error: unrecognized opcode: `movq'
dw2-restrict.c:59: Error: unrecognized opcode: `movsbl'
dw2-restrict.c:60: Error: unrecognized opcode: `ret'
dw2-restrict.c:76: Error: unrecognized opcode: `pushq'
dw2-restrict.c:78: Error: unrecognized opcode: `movq'
dw2-restrict.c:80: Error: unrecognized opcode: `subq'
dw2-restrict.c:82: Error: unrecognized opcode: `leaq'
dw2-restrict.c:83: Error: unrecognized opcode: `movl'
dw2-restrict.c:86: Error: unrecognized opcode: `callq'
dw2-restrict.c:87: Error: unrecognized opcode: `addq'
dw2-restrict.c:88: Error: unrecognized opcode: `popq'
dw2-restrict.c:89: Error: unrecognized opcode: `ret'
on ppc64le, 
dw2-restrict.c: Assembler messages:
dw2-restrict.c:55: Error: bad register name `%rdi'
dw2-restrict.c:58: Error: bad register name `%rsp)'
dw2-restrict.c:59: Error: bad register name `%rdi)'
dw2-restrict.c:76: Error: bad register name `%rbp'
dw2-restrict.c:78: Error: bad register name `%rsp'
dw2-restrict.c:80: Error: bad register name `%rsp'
dw2-restrict.c:82: Error: bad register name `%rdi'
dw2-restrict.c:83: Error: bad register name `%rbp)'
dw2-restrict.c:86: Error: invalid instruction suffix for `call'
dw2-restrict.c:87: Error: bad register name `%rsp'
dw2-restrict.c:88: Error: bad register name `%rbp'
...
/builddir/build/BUILD/dwz/testsuite/dwz.tests/dw2-skip-prologue.S:309: Error: cannot represent relocation type BFD_RELOC_64
as: /tmp/ccg3sw27.o: unsupported relocation type: 0x1
on i686 etc.  Can't we ensure that the tests are assembled only on the
corresponding architecture and nowhere else if they are architecture
specific?
It is fine if some tests are UNSUPPORTED, just we should completely skip
them if they can't be assembled.

	Jakub