Re: [PATCH v3 binutils] Add BPF support to binutils...

2017-05-01 Thread Alexei Starovoitov 

On 5/1/17 8:03 PM, David Miller wrote:

From: Alexei Starovoitov 
Date: Mon, 1 May 2017 19:49:21 -0700


On 4/30/17 11:21 AM, David Miller wrote:

built with:

clang -O2 -target bpfel -g -c x.c -o x.o

readelf can see it just fine:

[davem@localhost binutils]$ ./readelf --debug-dump=loc ./xel.o
Contents of the .debug_loc section:

Offset   BeginEnd  Expression
  0010 (DW_OP_reg1 (r1))
0013 
0023 0010 0020 (DW_OP_constu:
590618314553; DW_OP_stack_value)
003d 0020 0030 (DW_OP_reg1 (r1))
0050 

But with big-endian:

[davem@localhost binutils]$ ./readelf --debug-dump=loc ./xeb.o
readelf: Warning: Invalid pointer size (0) in compunit header, using 4
instead
readelf: Warning: Bogus end-of-siblings marker detected at offset 27
in .debug_info section
readelf: Warning: Bogus end-of-siblings marker detected at offset 28
in .debug_info section
readelf: Warning: DIE at offset 0x29 refers to abbreviation number 48
which does not exist
readelf: Warning: Unable to load/parse the .debug_info section, so
cannot interpret the .debug_loc section.


yeah. clang emitted dwarf for big-endian is broken.
This dwarf stuff is too complicated for normal human beings.
The tight packing making debugging it quite painful.


But doesn't the CLANG DWARF2 emission code look at the target
endianness?


it certainly does and on bpf backend side I'm not doing
anything special comparing to what other bi-endian architectures
like ppc and mips are doing. Obviously I missed something.

Re: [PATCH v3 binutils] Add BPF support to binutils...

2017-05-01 Thread David Miller 
From: Alexei Starovoitov 
Date: Mon, 1 May 2017 19:49:21 -0700

> On 4/30/17 11:21 AM, David Miller wrote:
>> built with:
>>
>>  clang -O2 -target bpfel -g -c x.c -o x.o
>>
>> readelf can see it just fine:
>>
>> [davem@localhost binutils]$ ./readelf --debug-dump=loc ./xel.o
>> Contents of the .debug_loc section:
>>
>> Offset   BeginEnd  Expression
>>   0010 (DW_OP_reg1 (r1))
>> 0013 
>> 0023 0010 0020 (DW_OP_constu:
>> 590618314553; DW_OP_stack_value)
>> 003d 0020 0030 (DW_OP_reg1 (r1))
>> 0050 
>>
>> But with big-endian:
>>
>> [davem@localhost binutils]$ ./readelf --debug-dump=loc ./xeb.o
>> readelf: Warning: Invalid pointer size (0) in compunit header, using 4
>> instead
>> readelf: Warning: Bogus end-of-siblings marker detected at offset 27
>> in .debug_info section
>> readelf: Warning: Bogus end-of-siblings marker detected at offset 28
>> in .debug_info section
>> readelf: Warning: DIE at offset 0x29 refers to abbreviation number 48
>> which does not exist
>> readelf: Warning: Unable to load/parse the .debug_info section, so
>> cannot interpret the .debug_loc section.
> 
> yeah. clang emitted dwarf for big-endian is broken.
> This dwarf stuff is too complicated for normal human beings.
> The tight packing making debugging it quite painful.

But doesn't the CLANG DWARF2 emission code look at the target
endianness?

Re: [PATCH v3 binutils] Add BPF support to binutils...

2017-05-01 Thread Alexei Starovoitov 

On 4/30/17 11:21 AM, David Miller wrote:

built with:

clang -O2 -target bpfel -g -c x.c -o x.o

readelf can see it just fine:

[davem@localhost binutils]$ ./readelf --debug-dump=loc ./xel.o
Contents of the .debug_loc section:

Offset   BeginEnd  Expression
  0010 (DW_OP_reg1 (r1))
0013 
0023 0010 0020 (DW_OP_constu: 590618314553; 
DW_OP_stack_value)
003d 0020 0030 (DW_OP_reg1 (r1))
0050 

But with big-endian:

[davem@localhost binutils]$ ./readelf --debug-dump=loc ./xeb.o
readelf: Warning: Invalid pointer size (0) in compunit header, using 4 instead
readelf: Warning: Bogus end-of-siblings marker detected at offset 27 in 
.debug_info section
readelf: Warning: Bogus end-of-siblings marker detected at offset 28 in 
.debug_info section
readelf: Warning: DIE at offset 0x29 refers to abbreviation number 48 which 
does not exist
readelf: Warning: Unable to load/parse the .debug_info section, so cannot 
interpret the .debug_loc section.


yeah. clang emitted dwarf for big-endian is broken.
This dwarf stuff is too complicated for normal human beings.
The tight packing making debugging it quite painful.

Re: [PATCH v3 binutils] Add BPF support to binutils...

2017-04-30 Thread David Miller 
From: Alexei Starovoitov 
Date: Sat, 29 Apr 2017 23:44:59 -0700

> '-g' still doesn't seem to work:
> /w/binutils-gdb/bld/binutils/objdump: invalid relocation type 10
> /w/binutils-gdb/bld/binutils/objdump: BFD (GNU Binutils)
> 2.28.51.20170429 assertion fail ../../bfd/elf64-bpf.c:139
>0: 18 01 00 00 39 47 98 83 ldimm64 r0, 590618314553

Ok, I can look at the debug info in little endian objects created by
clang now, but something is up with the dwarf information in
big-endian objects.

Your test program:

int bpf_prog1(void *ign)
{
volatile unsigned long t = 0x8983984739ull;
return *(unsigned long *)((0x8fff0002ull) + t);
}

built with:

clang -O2 -target bpfel -g -c x.c -o x.o

readelf can see it just fine:

[davem@localhost binutils]$ ./readelf --debug-dump=loc ./xel.o 
Contents of the .debug_loc section:

Offset   BeginEnd  Expression
  0010 (DW_OP_reg1 (r1))
0013 
0023 0010 0020 (DW_OP_constu: 590618314553; 
DW_OP_stack_value)
003d 0020 0030 (DW_OP_reg1 (r1))
0050 

But with big-endian:

[davem@localhost binutils]$ ./readelf --debug-dump=loc ./xeb.o 
readelf: Warning: Invalid pointer size (0) in compunit header, using 4 instead
readelf: Warning: Bogus end-of-siblings marker detected at offset 27 in 
.debug_info section
readelf: Warning: Bogus end-of-siblings marker detected at offset 28 in 
.debug_info section
readelf: Warning: DIE at offset 0x29 refers to abbreviation number 48 which 
does not exist
readelf: Warning: Unable to load/parse the .debug_info section, so cannot 
interpret the .debug_loc section.

GDB behaves similarly, xel.o works fine but for the big-endian object:

Reading symbols from ./xeb.o./../binutils-gdb/gdb/dwarf2read.c:16933: 
internal-error: read_address: bad switch, unsigned [in module 
/home/davem/src/GIT/BINUTILS/build-bpf/binutils/xeb.o]

It is entirely possible that the problem is on the LLVM side.
Can you double check that the dwarf2 emission code in LLVM is
using the correct endianness?

Here is my working diff against v4:

diff --git a/bfd/elf64-bpf.c b/bfd/elf64-bpf.c
index a42f768..1d8085e 100644
--- a/bfd/elf64-bpf.c
+++ b/bfd/elf64-bpf.c
@@ -15,6 +15,7 @@
 static reloc_howto_type _bfd_bpf_elf_howto_table[] =
 {
   HOWTO(R_BPF_NONE,  0,3, 0,FALSE,0,complain_overflow_dont,
bfd_elf_generic_reloc,  "R_BPF_NONE",FALSE,0,0x,TRUE),
+  HOWTO(R_BPF_DATA_64,   
0,4,64,FALSE,0,complain_overflow_bitfield,bfd_elf_generic_reloc,  
"R_BPF_DATA_64", FALSE,0,MINUS_ONE,TRUE),
 
   /* XXX these are wrong XXX */
   HOWTO(R_BPF_INSN_64,   
0,4,64,FALSE,0,complain_overflow_bitfield,bfd_elf_generic_reloc,  
"R_BPF_INSN_64", FALSE,0,MINUS_ONE,TRUE),
@@ -22,13 +23,11 @@ static reloc_howto_type _bfd_bpf_elf_howto_table[] =
   HOWTO(R_BPF_INSN_16,   
0,1,16,FALSE,0,complain_overflow_bitfield,bfd_elf_generic_reloc,  
"R_BPF_INSN_16", FALSE,0,0x,TRUE),
   HOWTO(R_BPF_WDISP16,   0,1,16,TRUE, 0,complain_overflow_signed,  
bfd_elf_generic_reloc,  "R_BPF_WDISP16", FALSE,0,0x,TRUE),
 
-  EMPTY_HOWTO(5),
   EMPTY_HOWTO(6),
   EMPTY_HOWTO(7),
   HOWTO(R_BPF_DATA_8,0,0, 
8,FALSE,0,complain_overflow_bitfield,bfd_elf_generic_reloc,  "R_BPF_DATA_8",  
FALSE,0,0x00ff,TRUE),
   HOWTO(R_BPF_DATA_16,   
0,1,16,FALSE,0,complain_overflow_bitfield,bfd_elf_generic_reloc,  
"R_BPF_DATA_16", FALSE,0,0x,TRUE),
   HOWTO(R_BPF_DATA_32,   
0,2,32,FALSE,0,complain_overflow_bitfield,bfd_elf_generic_reloc,  
"R_BPF_DATA_32", FALSE,0,0x,TRUE),
-  HOWTO(R_BPF_DATA_64,   
0,4,64,FALSE,0,complain_overflow_bitfield,bfd_elf_generic_reloc,  
"R_BPF_DATA_64", FALSE,0,MINUS_ONE,TRUE),
 };
 
 reloc_howto_type *
diff --git a/binutils/readelf.c b/binutils/readelf.c
index b4013fb..0e9716b 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -12254,7 +12254,7 @@ is_64bit_abs_reloc (unsigned int reloc_type)
 case EM_ALPHA:
   return reloc_type == 2; /* R_ALPHA_REFQUAD.  */
 case EM_BPF:
-  return reloc_type == 11; /* R_BPF_DATA_64 */
+  return reloc_type == 1; /* R_BPF_DATA_64 */
 case EM_IA_64:
   return reloc_type == 0x27; /* R_IA64_DIR64LSB.  */
 case EM_PARISC:
diff --git a/include/elf/bpf.h b/include/elf/bpf.h
index 4aa38cc..0d6fddc 100644
--- a/include/elf/bpf.h
+++ b/include/elf/bpf.h
@@ -26,14 +26,14 @@
 /* Relocation types.  */
 START_RELOC_NUMBERS (elf_bpf_reloc_type)
   RELOC_NUMBER (R_BPF_NONE, 0)
-  RELOC_NUMBER (R_BPF_INSN_64, 1)
-  RELOC_NUMBER (R_BPF_INSN_32, 2)
-  RELOC_NUMBER (R_BPF_INSN_16, 3)
-  RELOC_NUMBER (R_BPF_WDISP16, 4)
+  RELOC_NUMBER (R_BPF_DATA_64, 1)
+  RELOC_NUMBER (R_BPF_INSN_64, 2)
+  RELOC_NUMBER (R_BPF_INSN_32, 3)
+  RELOC_NUMBER (R_BPF_INSN_16, 4)
+  RELOC_NUMBER (R_BPF_WDISP16, 5)
   RELOC_NUMBER (R_BPF_DATA_8,  8)
   RELOC_NUMBER (R_BPF_DATA_16, 9)
   RELOC_NUMBER (R_BPF_DATA_32, 10)
-

Re: [PATCH v3 binutils] Add BPF support to binutils...

2017-04-30 Thread David Miller 
From: Alexei Starovoitov 
Date: Sat, 29 Apr 2017 23:44:59 -0700

> On 4/29/17 7:37 PM, David Miller wrote:
>> BTW, should I just remove tailcall from the opcode table altogether?
> 
> yeah. tailcall is not a special opcode from user space point of view.
> Only after normal call with func_id=bpf_tail_call passes verifier
> then verifier will change insn->code into CALL|X
> It's done only to have two 'case' statement in the interpreter,
> so that normal calls and tailcalls don't interfere.
> From user space pov CALL|X opcode is reserved and we can use it
> for something in the future. Just need to change interpeter and JITs.

Ok, I've removed it from my tree.

Thanks.

Re: [PATCH v3 binutils] Add BPF support to binutils...

2017-04-30 Thread David Miller 
From: Alexei Starovoitov 
Date: Sat, 29 Apr 2017 23:44:59 -0700

> On 4/29/17 7:37 PM, David Miller wrote:
>> From: David Miller 
>> Date: Sat, 29 Apr 2017 22:24:50 -0400 (EDT)
>>
>>> Some of your bugs should be fixed by this patch below, I'll add
>>> test cases soon:
>>
>> Ok, here are all the local changes in my tree.  I made the relocs
>> match LLVM and I fixed some dwarf debugging stuff.
>>
>> With this we are also down to one test case failure under binutils/
>> and it's something weird with merging 64-bit notes which I should be
>> able to fix soon.
>>
>> I can fix these bugs fast, keep reporting.
>>
>> BTW, should I just remove tailcall from the opcode table altogether?
> 
> yeah. tailcall is not a special opcode from user space point of view.
> Only after normal call with func_id=bpf_tail_call passes verifier
> then verifier will change insn->code into CALL|X
> It's done only to have two 'case' statement in the interpreter,
> so that normal calls and tailcalls don't interfere.
> From user space pov CALL|X opcode is reserved and we can use it
> for something in the future. Just need to change interpeter and JITs.
> 
>>  case 'O':
>> -  (*info->fprintf_func) (stream, "%d", off);
>> +  (*info->fprintf_func) (stream, "%d", (int) off);
> 
> tried this diff. It looks better
>   10: 7b 1a f8 ff 00 00 00 00 stdw[r1+-8], r10
>   18: 79 a1 f8 ff 00 00 00 00 lddwr10, [r1+-8]
> I wonder if '+' can be removed as well.

All disassemblers in binutils print it this way, sparc, x86, etc.

> '-g' still doesn't seem to work:
> /w/binutils-gdb/bld/binutils/objdump: invalid relocation type 10
> /w/binutils-gdb/bld/binutils/objdump: BFD (GNU Binutils)
> 2.28.51.20170429 assertion fail ../../bfd/elf64-bpf.c:139
>0: 18 01 00 00 39 47 98 83 ldimm64 r0, 590618314553

Hmm, I defined a relocation type 10 in the patch, make sure BFD got
rebuilt properly...

I'll double check here too.

Re: [PATCH v3 binutils] Add BPF support to binutils...

2017-04-30 Thread Alexei Starovoitov 

On 4/29/17 7:37 PM, David Miller wrote:

From: David Miller 
Date: Sat, 29 Apr 2017 22:24:50 -0400 (EDT)


Some of your bugs should be fixed by this patch below, I'll add
test cases soon:


Ok, here are all the local changes in my tree.  I made the relocs
match LLVM and I fixed some dwarf debugging stuff.

With this we are also down to one test case failure under binutils/
and it's something weird with merging 64-bit notes which I should be
able to fix soon.

I can fix these bugs fast, keep reporting.

BTW, should I just remove tailcall from the opcode table altogether?


yeah. tailcall is not a special opcode from user space point of view.
Only after normal call with func_id=bpf_tail_call passes verifier
then verifier will change insn->code into CALL|X
It's done only to have two 'case' statement in the interpreter,
so that normal calls and tailcalls don't interfere.
From user space pov CALL|X opcode is reserved and we can use it
for something in the future. Just need to change interpeter and JITs.


case 'O':
- (*info->fprintf_func) (stream, "%d", off);
+ (*info->fprintf_func) (stream, "%d", (int) off);


tried this diff. It looks better
  10:   7b 1a f8 ff 00 00 00 00 stdw[r1+-8], r10
  18:   79 a1 f8 ff 00 00 00 00 lddwr10, [r1+-8]
I wonder if '+' can be removed as well.

'-g' still doesn't seem to work:
/w/binutils-gdb/bld/binutils/objdump: invalid relocation type 10
/w/binutils-gdb/bld/binutils/objdump: BFD (GNU Binutils) 
2.28.51.20170429 assertion fail ../../bfd/elf64-bpf.c:139

   0:   18 01 00 00 39 47 98 83 ldimm64 r0, 590618314553

Re: [PATCH v3 binutils] Add BPF support to binutils...

2017-04-29 Thread David Miller 
From: David Miller 
Date: Sat, 29 Apr 2017 22:24:50 -0400 (EDT)

> Some of your bugs should be fixed by this patch below, I'll add
> test cases soon:

Ok, here are all the local changes in my tree.  I made the relocs
match LLVM and I fixed some dwarf debugging stuff.

With this we are also down to one test case failure under binutils/
and it's something weird with merging 64-bit notes which I should be
able to fix soon.

I can fix these bugs fast, keep reporting.

BTW, should I just remove tailcall from the opcode table altogether?

diff --git a/binutils/readelf.c b/binutils/readelf.c
index 6c67d98..3931a3a 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -95,6 +95,7 @@
 #include "elf/arm.h"
 #include "elf/avr.h"
 #include "elf/bfin.h"
+#include "elf/bpf.h"
 #include "elf/cr16.h"
 #include "elf/cris.h"
 #include "elf/crx.h"
@@ -746,6 +747,7 @@ guess_is_rela (unsigned int e_machine)
 case EM_AVR:
 case EM_AVR_OLD:
 case EM_BLACKFIN:
+case EM_BPF:
 case EM_CR16:
 case EM_CRIS:
 case EM_CRX:
@@ -1458,6 +1460,10 @@ dump_relocations (FILE * file,
  rtype = elf_bfin_reloc_type (type);
  break;
 
+   case EM_BPF:
+ rtype = elf_bpf_reloc_type (type);
+ break;
+
case EM_CYGNUS_MEP:
  rtype = elf_mep_reloc_type (type);
  break;
diff --git a/gas/config/tc-bpf.c b/gas/config/tc-bpf.c
index 0ba2afa..36393b7 100644
--- a/gas/config/tc-bpf.c
+++ b/gas/config/tc-bpf.c
@@ -288,6 +288,14 @@ md_assemble (char *str ATTRIBUTE_UNUSED)
  switch (*args)
{
case '+':
+ if (*s == '+')
+   {
+ ++s;
+ continue;
+   }
+ if (*s == '-')
+   continue;
+ break;
case ',':
case '[':
case ']':
@@ -494,6 +502,9 @@ md_apply_fix (fixS *fixP, valueT *valP ATTRIBUTE_UNUSED, 
segT segment ATTRIBUTE_
   offsetT val = * (offsetT *) valP;
 
   gas_assert (fixP->fx_r_type < BFD_RELOC_UNUSED);
+
+  fixP->fx_addnumber = val;/* Remember value for emit_reloc.  */
+
   /* If this is a data relocation, just output VAL.  */
 
   if (fixP->fx_r_type == BFD_RELOC_8)
diff --git a/include/elf/bpf.h b/include/elf/bpf.h
index 5019b11..77463e3 100644
--- a/include/elf/bpf.h
+++ b/include/elf/bpf.h
@@ -26,14 +26,14 @@
 /* Relocation types.  */
 START_RELOC_NUMBERS (elf_bpf_reloc_type)
   RELOC_NUMBER (R_BPF_NONE, 0)
-  RELOC_NUMBER (R_BPF_INSN_16, 1)
-  RELOC_NUMBER (R_BPF_INSN_32, 2)
-  RELOC_NUMBER (R_BPF_INSN_64, 3)
-  RELOC_NUMBER (R_BPF_WDISP16, 4)
-  RELOC_NUMBER (R_BPF_DATA_8,  5)
-  RELOC_NUMBER (R_BPF_DATA_16, 6)
-  RELOC_NUMBER (R_BPF_DATA_32, 7)
-  RELOC_NUMBER (R_BPF_DATA_64, 8)
+  RELOC_NUMBER (R_BPF_DATA_64, 1)
+  RELOC_NUMBER (R_BPF_INSN_16, 2)
+  RELOC_NUMBER (R_BPF_INSN_32, 3)
+  RELOC_NUMBER (R_BPF_INSN_64, 4)
+  RELOC_NUMBER (R_BPF_WDISP16, 5)
+  RELOC_NUMBER (R_BPF_DATA_8,  6)
+  RELOC_NUMBER (R_BPF_DATA_16, 7)
+  RELOC_NUMBER (R_BPF_DATA_32, 10)
 END_RELOC_NUMBERS (R_BPF_max)
 
 #endif /* _ELF_BPF_H */
diff --git a/opcodes/bpf-dis.c b/opcodes/bpf-dis.c
index 92e29af..39656bf 100644
--- a/opcodes/bpf-dis.c
+++ b/opcodes/bpf-dis.c
@@ -49,7 +49,7 @@ print_insn_bpf (bfd_vma memaddr, disassemble_info *info)
   bpf_opcode_hash *op;
   int code, dest, src;
   bfd_byte buffer[8];
-  unsigned short off;
+  signed short off;
   int status, ret;
   signed int imm;
 
@@ -78,7 +78,7 @@ print_insn_bpf (bfd_vma memaddr, disassemble_info *info)
   else
 {
   getword = bfd_getl32;
-  gethalf = bfd_getl32;
+  gethalf = bfd_getl16;
 }  
 
   code = buffer[0];
@@ -128,7 +128,7 @@ print_insn_bpf (bfd_vma memaddr, disassemble_info *info)
  (*info->fprintf_func) (stream, "%d", imm);
  break;
case 'O':
- (*info->fprintf_func) (stream, "%d", off);
+ (*info->fprintf_func) (stream, "%d", (int) off);
  break;
case 'L':
  info->target = memaddr + ((off - 1) * 8);

Re: [PATCH v3 binutils] Add BPF support to binutils...

2017-04-29 Thread Alexei Starovoitov 

On 4/29/17 7:13 PM, David Miller wrote:

From: Alexei Starovoitov 
Date: Sat, 29 Apr 2017 17:48:43 -0700


/w/binutils-gdb/bld/binutils/objdump: invalid relocation type 10
/w/binutils-gdb/bld/binutils/objdump: Dwarf Error: found address size


I discussed this in another email, the relocation numbers I used in
binutils do not match what is in LLVM currently.

In fact, I thought you guys weren't using relocations in any capacity
at all so just picked things from scratch :-)


yeah :) will reply in the other thread.
Too many public and internal discussions in the last week.
Weekend is the only time to reduce the backlog :)

> Please use "--target=bpf-elf"

Thanks. That worked. Built the whole thing :)

objdump behaves the same way.
When compiled by clang with '-g'
(gdb) x/10i bpf_prog1
   0x0 : 
ldimm64	r0, 590618314553
   0x10 : 
stdw	[r1+65528], r10
   0x18 : 
lddw	r10, [r1+65528]
   0x20 : 
add	r0, -1879113726
   0x28 : 
lddw	r1, [r0+0]

   0x30 :  exit
   0x38:Cannot access memory at address 0x38

Even without -g the last line 'Cannot access' is printed.
It seems gdb miscalculates the total func size?
The printing of 'clang version...' is due to '-g'.
Without -g it looks good:
(gdb) x/10i bpf_prog1
   0x0 :   ldimm64 r1, 590618314553
   0x10 :   stdw[r10+65528], r1

Btw, I'm using this C file for testing:
int bpf_prog1(void *ign)
{
  volatile unsigned long t = 0x8983984739ull;
  return *(unsigned long *)((0x8fff0002ull) + t);
}

There was a bug in llvm backend with imm overflow which
was recently fixed.

Re: [PATCH v3 binutils] Add BPF support to binutils...

2017-04-29 Thread David Miller 
From: Alexei Starovoitov 
Date: Sat, 29 Apr 2017 17:48:43 -0700

> $ bld/binutils/objdump -S test.o
> 
> test.o: file format elf64-bpfbe
> 
> Disassembly of section .text:
> 
>  :
>0: 18 10 00 00 83 98 47 39 ldimm64 r1, 590618314553
>8: 00 00 00 00 00 00 00 89
>   10: 7b a1 ff f8 00 00 00 00 stdw[r10+65528], r1
>   18: 79 1a ff f8 00 00 00 00 lddwr1, [r10+65528]
>   20: 07 10 00 00 8f ff 00 02 add r1, -1879113726
>   28: 79 01 00 00 00 00 00 00 lddwr0, [r1+0]
>   30: 95 00 00 00 00 00 00 00 exit
> 
> looks good except negative offsets are reported as large positive.

Some of your bugs should be fixed by this patch below, I'll add
test cases soon:

diff --git a/gas/config/tc-bpf.c b/gas/config/tc-bpf.c
index 0ba2afa..36393b7 100644
--- a/gas/config/tc-bpf.c
+++ b/gas/config/tc-bpf.c
@@ -288,6 +288,14 @@ md_assemble (char *str ATTRIBUTE_UNUSED)
  switch (*args)
{
case '+':
+ if (*s == '+')
+   {
+ ++s;
+ continue;
+   }
+ if (*s == '-')
+   continue;
+ break;
case ',':
case '[':
case ']':
diff --git a/opcodes/bpf-dis.c b/opcodes/bpf-dis.c
index 92e29af..39656bf 100644
--- a/opcodes/bpf-dis.c
+++ b/opcodes/bpf-dis.c
@@ -49,7 +49,7 @@ print_insn_bpf (bfd_vma memaddr, disassemble_info *info)
   bpf_opcode_hash *op;
   int code, dest, src;
   bfd_byte buffer[8];
-  unsigned short off;
+  signed short off;
   int status, ret;
   signed int imm;
 
@@ -78,7 +78,7 @@ print_insn_bpf (bfd_vma memaddr, disassemble_info *info)
   else
 {
   getword = bfd_getl32;
-  gethalf = bfd_getl32;
+  gethalf = bfd_getl16;
 }  
 
   code = buffer[0];
@@ -128,7 +128,7 @@ print_insn_bpf (bfd_vma memaddr, disassemble_info *info)
  (*info->fprintf_func) (stream, "%d", imm);
  break;
case 'O':
- (*info->fprintf_func) (stream, "%d", off);
+ (*info->fprintf_func) (stream, "%d", (int) off);
  break;
case 'L':
  info->target = memaddr + ((off - 1) * 8);

Re: [PATCH v3 binutils] Add BPF support to binutils...

2017-04-29 Thread David Miller 
From: Alexei Starovoitov 
Date: Sat, 29 Apr 2017 17:48:43 -0700

> /w/binutils-gdb/bld/binutils/objdump: invalid relocation type 10
> /w/binutils-gdb/bld/binutils/objdump: Dwarf Error: found address size

I discussed this in another email, the relocation numbers I used in
binutils do not match what is in LLVM currently.

In fact, I thought you guys weren't using relocations in any capacity
at all so just picked things from scratch :-)

Re: [PATCH v3 binutils] Add BPF support to binutils...

2017-04-29 Thread David Miller 
From: Alexei Starovoitov 
Date: Sat, 29 Apr 2017 17:48:43 -0700

> On 4/28/17 1:33 PM, David Miller wrote:
>> New in this version:
>>
>> 1) All the relocation work I posted earlier today.
>> 2) Teach readelf about a few bpf relocs as needed
>> 3) Add a 'nop' instruction which facilitates the gas
>>testsuite.  I used "mov r0,r0"
>>
>> The whole gas testsuite passes now. :-)  But that just
>> means we have to add more tests I guess
>>
>> Signed-off-by: David S. Miller 
> 
> it seems by default bpf target is not enabled,
> so I tried to build it with:
> ../configure --enable-targets=bpf,x86

Please use "--target=bpf-elf"

Re: [PATCH v3 binutils] Add BPF support to binutils...

2017-04-29 Thread Alexei Starovoitov 

On 4/28/17 1:33 PM, David Miller wrote:

New in this version:

1) All the relocation work I posted earlier today.
2) Teach readelf about a few bpf relocs as needed
3) Add a 'nop' instruction which facilitates the gas
   testsuite.  I used "mov r0,r0"

The whole gas testsuite passes now. :-)  But that just
means we have to add more tests I guess

Signed-off-by: David S. Miller 


it seems by default bpf target is not enabled,
so I tried to build it with:
../configure --enable-targets=bpf,x86
make -j40
but it failed to build :(

Then I did ../configure --target=bpf
and it went better. At least I could compile objdump,
but gas refused to be configured:
checking whether byte ordering is bigendian... no
This target is no longer supported in gas
make[1]: *** [configure-gas] Error 1

Not sure what I'm doing wrong.

At least i tested objdump:
$ clang -O2 -target bpfeb -c test.c
$ llvm-objdump -S test.o

test.o: file format ELF64-BPF

Disassembly of section .text:
bpf_prog1:
   0:	18 10 00 00 83 98 47 39 00 00 00 00 00 00 00 89 	r1 = 
590618314553ll

   2:   7b a1 ff f8 00 00 00 00 *(u64 *)(r10 - 8) = r1
   3:   79 1a ff f8 00 00 00 00 r1 = *(u64 *)(r10 - 8)
   4:   07 10 00 00 8f ff 00 02 r1 += -1879113726
   5:   79 01 00 00 00 00 00 00 r0 = *(u64 *)(r1 + 0)
   6:   95 00 00 00 00 00 00 00 exit

$ bld/binutils/objdump -S test.o

test.o: file format elf64-bpfbe

Disassembly of section .text:

 :
   0:   18 10 00 00 83 98 47 39 ldimm64 r1, 590618314553
   8:   00 00 00 00 00 00 00 89
  10:   7b a1 ff f8 00 00 00 00 stdw[r10+65528], r1
  18:   79 1a ff f8 00 00 00 00 lddwr1, [r10+65528]
  20:   07 10 00 00 8f ff 00 02 add r1, -1879113726
  28:   79 01 00 00 00 00 00 00 lddwr0, [r1+0]
  30:   95 00 00 00 00 00 00 00 exit

looks good except negative offsets are reported as large positive.

If compiled with clang -O2 -target bpfel and result is:
$ bld/binutils/objdump -S test.o

test.o: file format elf64-bpfle

Disassembly of section .text:

 :
   0:   18 01 00 00 39 47 98 83 ldimm64 r0, 590618314553
   8:   00 00 00 00 89 00 00 00
  10:   7b 1a f8 ff 00 00 00 00 stdw[r1+65528], r10
  18:   79 a1 f8 ff 00 00 00 00 lddwr10, [r1+65528]

Now the src/dst registers are swapped :(
The bytes printed are correct though.

clang debug info is not recognized as well:
$ clang -O2 -g -target bpfel -c test.c
/w/binutils-gdb/bld/binutils/objdump -S test.o

test.o: file format elf64-bpfle


Disassembly of section .text:

 :
/w/binutils-gdb/bld/binutils/objdump: invalid relocation type 10
/w/binutils-gdb/bld/binutils/objdump: BFD (GNU Binutils) 
2.28.51.20170429 assertion fail ../../bfd/elf64-bpf.c:139

...
/w/binutils-gdb/bld/binutils/objdump: BFD (GNU Binutils) 
2.28.51.20170429 assertion fail ../../bfd/elf64-bpf.c:139

/w/binutils-gdb/bld/binutils/objdump: invalid relocation type 10
/w/binutils-gdb/bld/binutils/objdump: BFD (GNU Binutils) 
2.28.51.20170429 assertion fail ../../bfd/elf64-bpf.c:139

   0:   18 01 00 00 39 47 98 83 ldimm64 r0, 590618314553
   8:   00 00 00 00 89 00 00 00
  10:   7b 1a f8 ff 00 00 00 00 stdw[r1+65528], r10

$ clang -O2 -g -target bpfeb -c test.c
$ /w/binutils-gdb/bld/binutils/objdump -S test.o

test.o: file format elf64-bpfbe

Disassembly of section .text:

 :
/w/binutils-gdb/bld/binutils/objdump: invalid relocation type 10
/w/binutils-gdb/bld/binutils/objdump: BFD (GNU Binutils) 
2.28.51.20170429 assertion fail ../../bfd/elf64-bpf.c:139

...
/w/binutils-gdb/bld/binutils/objdump: invalid relocation type 10
/w/binutils-gdb/bld/binutils/objdump: Dwarf Error: found address size 
'0', this reader can only handle address sizes '2', '4' and '8'.

   0:   18 10 00 00 83 98 47 39 ldimm64 r1, 590618314553
   8:   00 00 00 00 00 00 00 89
  10:   7b a1 ff f8 00 00 00 00 stdw[r10+65528], r1
  18:   79 1a ff f8 00 00 00 00 lddwr1, [r10+65528]

With llvm it should be like this:
$ ./bin/clang -O2 -g -target bpfel -c test.c
$ ./bin/llvm-objdump -S test.o

test.o: file format ELF64-BPF

Disassembly of section .text:
bpf_prog1:
; {
   0:	18 01 00 00 39 47 98 83 00 00 00 00 89 00 00 00 	r1 = 
590618314553ll

; volatile unsigned long t = 0x8983984739ull;
   2:   7b 1a f8 ff 00 00 00 00 *(u64 *)(r10 - 8) = r1
; return *(unsigned long *)((0x8fff0002ull) + t);
   3:   79 a1 f8 ff 00 00 00 00 r1 = *(u64 *)(r10 - 8)
   4:   07 01 00 00 02 00 ff 8f r1 += -1879113726
   5:   79 10 00 00 00 00 00 00 r0 = *(u64 *)(r1 + 0)
   6:   95 00 00 00 00 00 00 00 exit

The output of llvm with '-g -target bpfeb' is probably partially broken,
since 'llvm-objdump -S test.o' doesn't see any debug in there.
So gnu objdump's