Re: clang generated code sometimes confuses fbt

2013-03-05 Thread Andriy Gapon
on 04/03/2013 13:50 Dimitry Andric said the following:
 Actually, that way of fixing breaks it for gcc, so it should really be
 fixed in sort_iidescs() instead.  See attached diff; I verified it works
 for both gcc-compiled and clang-compiled objects.

Impossible!  It looks like the attached diff was a nop change :-)

BTW, so changing clang to match gcc is not an option?
I do not insist, just curious.

-- 
Andriy Gapon
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


Re: clang generated code sometimes confuses fbt

2013-03-05 Thread Dimitry Andric

On 2013-03-05 13:02, Andriy Gapon wrote:

on 04/03/2013 13:50 Dimitry Andric said the following:

Actually, that way of fixing breaks it for gcc, so it should really be
fixed in sort_iidescs() instead.  See attached diff; I verified it works
for both gcc-compiled and clang-compiled objects.


Impossible!  It looks like the attached diff was a nop change :-)

BTW, so changing clang to match gcc is not an option?
I do not insist, just curious.


I could not find a good argument to present to upstream to change this
behaviour, except gcc has been doing it for x years, without a good
reason. :-)  And I did not want to make yet another FreeBSD specific
change.

In my opinion, gcc is losing information here, e.g. the original
pathname to the source file.  There is no reason debugging tools cannot
be fixed to be able to cope with this.
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


Re: clang generated code sometimes confuses fbt

2013-03-04 Thread Dimitry Andric

On 2013-03-04 08:57, Matt Burke wrote:

On 03/02/13 17:35, Andriy Gapon wrote:

To summarize: I would be glad of either clang generated code was
fbt-friendly or if ctf information was generated for
bpobj_iterate_impl. Either is perfect for me.


Apologies if this is a silly suggestion, but are you building with -O0?
I've noticed backtraces being hard to follow (i.e. skipping intermediate
and wrapper functions) with a kernel built with the default optimisation
level, and it seems to me that the same thing may be happening here - the
wrappers are being optimised out.


No, the wrappers (bpobj_iterate and bpobj_iterate_nofree) are externally
visible functions, so they cannot be optimized out.  Both of them call
bpobj_iterate_impl, which is a static function, e.g. a candidate for
automatic inlining.

However, bpobj_iterate_impl is quite big, and called from 3 different
locations, so it does not actually get inlined, even with -O2.
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


Re: clang generated code sometimes confuses fbt

2013-03-04 Thread Andriy Gapon
on 04/03/2013 09:57 Matt Burke said the following:
 On 03/02/13 17:35, Andriy Gapon wrote:
 
 To summarize: I would be glad of either clang generated code was
 fbt-friendly or if ctf information was generated for
 bpobj_iterate_impl. Either is perfect for me.
 
 Apologies if this is a silly suggestion,

Apologies, but yes, it is :-)

 but are you building with -O0?
 I've noticed backtraces being hard to follow (i.e. skipping intermediate
 and wrapper functions) with a kernel built with the default optimisation
 level, and it seems to me that the same thing may be happening here - the
 wrappers are being optimised out.
 

If they were optimized out, then I wouldn't be able to provide their disassembly
in my original email, would I?


-- 
Andriy Gapon
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


Re: clang generated code sometimes confuses fbt

2013-03-04 Thread Dimitry Andric

On 2013-03-03 21:02, Dimitry Andric wrote:

On 2013-03-03 17:36, Andriy Gapon wrote:

on 03/03/2013 17:45 Dimitry Andric said the following:

...

So to fix this inconsistency, we could either change dw_read() to use
the full name instead of the basename, or change sort_iidescs() to use
the basename instead of the full name.

The only upstream I have been able to find, Illumos, had the same code
as FreeBSD for both of these functions, so it is likely they never
noticed it before...

My preference would be to remove the basename() call from dw_read(), as
in the attached diff.  That seems to fix the problem here; the diff of
ctfdump output before and after becomes:


Actually, that way of fixing breaks it for gcc, so it should really be
fixed in sort_iidescs() instead.  See attached diff; I verified it works
for both gcc-compiled and clang-compiled objects.
Index: cddl/contrib/opensolaris/tools/ctf/cvt/output.c
===
--- cddl/contrib/opensolaris/tools/ctf/cvt/output.c	(revision 247448)
+++ cddl/contrib/opensolaris/tools/ctf/cvt/output.c	(working copy)
@@ -363,6 +363,7 @@ sort_iidescs(Elf *elf, const char *file, tdata_t *
 
 	for (i = 0; i  nent; i++) {
 		GElf_Sym sym;
+		char *bname;
 		iidesc_t **tolist;
 		GElf_Sym ssym;
 		iidesc_match_t smatch;
@@ -377,6 +378,8 @@ sort_iidescs(Elf *elf, const char *file, tdata_t *
 
 		switch (GELF_ST_TYPE(sym.st_info)) {
 		case STT_FILE:
+			bname = strrchr(match.iim_name, '/');
+			bname = bname == NULL ? match.iim_name : bname + 1;
 			match.iim_file = match.iim_name;
 			continue;
 		case STT_OBJECT:
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org

Re: clang generated code sometimes confuses fbt

2013-03-03 Thread Dimitry Andric

On 2013-03-02 21:23, Dimitry Andric wrote:

On 2013-03-02 18:52, Andriy Gapon wrote:

on 02/03/2013 19:35 Andriy Gapon said the following:

Now, I am not quite sure why ctfconvert skips bpobj_iterate_impl in the
clang-generated code.  Seems like some sort of a bug in ctfconvert.


It seems that gcc and clang put different names for symbol of type FILE:
clang:
readelf -a -W /usr/obj/usr/src/sys/TRANT/bpobj.o| fgrep -w FILE
   1:  0 FILELOCAL  DEFAULT  ABS
/usr/src/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/bpobj.c

gcc:
readelf -a -W /usr/obj/usr/src/sys/ODYSSEY/bpobj.o| fgrep -w FILE
   1:  0 FILELOCAL  DEFAULT  ABS bpobj.c

ctfconvert seems to compare this value with bpobj.c and so in the clang case
it doesn't recognize the static symbols.

Does my analysis seem reasonable?


Have you verified that ctfconvert does the right thing, if you modify
the FILE symbol to have just the filename?


Debug log of ctfconvert operating on a gcc-compiled bpobj.o:

  http://www.andric.com/freebsd/ctfconvert-bpobj-gcc.log

The same, but on a clang-compiled bpobj.o:

  http://www.andric.com/freebsd/ctfconvert-bpobj-clang.log

This latter log does not change at all, if you change the FILE symbol to
just bpobj.c.  So there seems to something else in (probably) the
debug information that confuses ctfconvert.
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


Re: clang generated code sometimes confuses fbt

2013-03-03 Thread Andriy Gapon
on 02/03/2013 22:23 Dimitry Andric said the following:
 
 Have you verified that ctfconvert does the right thing, if you modify
 the FILE symbol to have just the filename?

No, I haven't.
How can I test that?

However my reading of the code makes me believe that that would help.

-- 
Andriy Gapon
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


Re: clang generated code sometimes confuses fbt

2013-03-03 Thread Andriy Gapon
on 03/03/2013 17:45 Dimitry Andric said the following:
 Debug log of ctfconvert operating on a gcc-compiled bpobj.o:
 
   http://www.andric.com/freebsd/ctfconvert-bpobj-gcc.log
 
 The same, but on a clang-compiled bpobj.o:
 
   http://www.andric.com/freebsd/ctfconvert-bpobj-clang.log
 
 This latter log does not change at all, if you change the FILE symbol to
 just bpobj.c.  So there seems to something else in (probably) the
 debug information that confuses ctfconvert.

Sorry, but these two logs look so different to each other and your clang log is
so different from a log that I get here with that I believe that something is
wrong in your test.

-- 
Andriy Gapon
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


Re: clang generated code sometimes confuses fbt

2013-03-03 Thread Dimitry Andric

On 2013-03-03 17:36, Andriy Gapon wrote:

on 03/03/2013 17:45 Dimitry Andric said the following:

Debug log of ctfconvert operating on a gcc-compiled bpobj.o:

   http://www.andric.com/freebsd/ctfconvert-bpobj-gcc.log

The same, but on a clang-compiled bpobj.o:

   http://www.andric.com/freebsd/ctfconvert-bpobj-clang.log

This latter log does not change at all, if you change the FILE symbol to
just bpobj.c.  So there seems to something else in (probably) the
debug information that confuses ctfconvert.


Sorry, but these two logs look so different to each other and your clang log is
so different from a log that I get here with that I believe that something is
wrong in your test.


Indeed, apparently I copy/pasted the wrong command line from earlier in
the build log, where it does not use -g (it is probably for the userland
version of ctf).

When I used the command line for the kernel build of bpobj.c, it did
have -g, and the ctfconvert log had a lot more information.  I have
re-uploaded the logfile on the second URL above.

I also had a look at ctfconvert, and it appears that in its dwarf
reader, in cddl/contrib/opensolaris/tools/ctf/cvt/dwarf.c, it uses the
basename, see lines 1825 through 1827:

  1762  int
  1763  dw_read(tdata_t *td, Elf *elf, char *filename __unused)
  1764  {
[...]
  1824  if ((dw.dw_cuname = die_name(dw, cu)) != NULL) {
  1825  char *base = xstrdup(basename(dw.dw_cuname));
  1826  free(dw.dw_cuname);
  1827  dw.dw_cuname = base;
  1828  
  1829  debug(1, CU name: %s\n, dw.dw_cuname);
  1830  }

while later on, in actually outputting the processed symbol table, in
cddl/contrib/opensolaris/tools/ctf/cvt/output.c, it does not take the
basename, but the full name:

   336  static iiburst_t *
   337  sort_iidescs(Elf *elf, const char *file, tdata_t *td, int fuzzymatch,
   338  int dynsym)
   339  {
[...]
   364  for (i = 0; i  nent; i++) {
   365  GElf_Sym sym;
[...]
   372  if (gelf_getsym(data, i, sym) == NULL)
   373  elfterminate(file, Couldn't read symbol %d, 
i);
   374  
   375  match.iim_name = (char *)strdata-d_buf + sym.st_name;
   376  match.iim_bind = GELF_ST_BIND(sym.st_info);
   377  
   378  switch (GELF_ST_TYPE(sym.st_info)) {
   379  case STT_FILE:
   380  match.iim_file = match.iim_name;
   381  continue;
[...]
   397  iidesc = find_iidesc(td, match);
   398  
   399  if (iidesc != NULL) {
   400  tolist[*curr] = iidesc;
   401  iidesc-ii_flags |= IIDESC_F_USED;
   402  (*curr)++;
   403  continue;
   404  }

On line 375, the full name is gotten from the elf symbol, on line 379 it
is assigned to the iidesc_match_t struct which is used for searching,
and in line 397 it does the actual search.

So to fix this inconsistency, we could either change dw_read() to use
the full name instead of the basename, or change sort_iidescs() to use
the basename instead of the full name.

The only upstream I have been able to find, Illumos, had the same code
as FreeBSD for both of these functions, so it is likely they never
noticed it before...

My preference would be to remove the basename() call from dw_read(), as
in the attached diff.  That seems to fix the problem here; the diff of
ctfdump output before and after becomes:

--- ctfdump-before.log  2013-03-03 20:37:30.0 +0100
+++ ctfdump-after.log   2013-03-03 20:37:47.0 +0100
@@ -9,8 +9,8 @@
   cth_lbloff   = 0
   cth_objtoff  = 8
   cth_funcoff  = 10
-  cth_typeoff  = 140
-  cth_stroff   = 24164
+  cth_typeoff  = 160
+  cth_stroff   = 24184
   cth_strlen   = 20066

 - Label Table 
@@ -23,6 +23,8 @@

 - Functions --

+  [0] FUNC (bpobj_iterate_impl) returns: 3 args: (1089, 1136, 56, 1092, 1163)
+  [1] FUNC (space_range_cb) returns: 3 args: (56, 1124, 1092)
   [2] FUNC (bpobj_alloc) returns: 324 args: (1080, 1092, 3)
   [3] FUNC (bpobj_alloc_empty) returns: 324 args: (1080, 3, 1092)
   [4] FUNC (bpobj_close) returns: 55 args: (1089)
@@ -5099,10 +5101,10 @@

   total number of data objects= 1

-  total number of functions   = 12
-  total number of function arguments  = 39
+  total number of functions   = 14
+  total number of function arguments  = 47
   maximum argument list length= 6
-  average argument list length= 3.25
+  average argument list length= 3.36

   total number of types   = 1173
   total number of integers= 18
Index: cddl/contrib/opensolaris/tools/ctf/cvt/dwarf.c

Re: clang generated code sometimes confuses fbt

2013-03-03 Thread Matt Burke
On 03/02/13 17:35, Andriy Gapon wrote:

 To summarize: I would be glad of either clang generated code was
 fbt-friendly or if ctf information was generated for
 bpobj_iterate_impl. Either is perfect for me.

Apologies if this is a silly suggestion, but are you building with -O0?
I've noticed backtraces being hard to follow (i.e. skipping intermediate
and wrapper functions) with a kernel built with the default optimisation
level, and it seems to me that the same thing may be happening here - the
wrappers are being optimised out.


-- 
Sorry for the following


iCritical is a brand of Critical Software Ltd. 
Registered in England  Wales: 04909220.
Registered Office: IC2, Keele Science Park, Keele, Staffordshire, ST5 5NH.

This message has been scanned for security threats by iCritical. 

The information contained in this message is confidential and intended for the 
addressee only. 
If you have received this message in error, or there are any problems with its 
content, please 
contact the sender.
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


clang generated code sometimes confuses fbt

2013-03-02 Thread Andriy Gapon

I observe the following problem.

There are two tiny wrapper functions around a larger implementation function:
int
bpobj_iterate(bpobj_t *bpo, bpobj_itor_t func, void *arg, dmu_tx_t *tx)
{
return (bpobj_iterate_impl(bpo, func, arg, tx, B_TRUE));
}
int
bpobj_iterate_nofree(bpobj_t *bpo, bpobj_itor_t func, void *arg, dmu_tx_t *tx)
{
return (bpobj_iterate_impl(bpo, func, arg, tx, B_FALSE));
}

On a clang compiled system:
$ dtrace -l | fgrep bpobj_iterate
 1483fbtkernelbpobj_iterate_impl entry
 1484fbtkernelbpobj_iterate_impl return

On a gcc compiled system:
dtrace -l | fgrep bpobj_iterate
  647fbtkernelbpobj_iterate_impl entry
  648fbtkernelbpobj_iterate_impl return
20656fbtkernel bpobj_iterate entry
20657fbtkernel bpobj_iterate return
28426fbtkernel  bpobj_iterate_nofree entry
28427fbtkernel  bpobj_iterate_nofree return

Examination reveals why that is so.
clang:
Dump of assembler code for function bpobj_iterate:
0x802d5a90 bpobj_iterate+0:   mov$0x1,%r8d
0x802d5a96 bpobj_iterate+6:   jmp0x802d5aa0
bpobj_iterate_impl

gcc:
Dump of assembler code for function bpobj_iterate:
0x802d3f43 bpobj_iterate+0:   push   %rbp
0x802d3f44 bpobj_iterate+1:   mov%rsp,%rbp
0x802d3f47 bpobj_iterate+4:   mov$0x1,%r8d
0x802d3f4d bpobj_iterate+10:  callq  0x802d3787
bpobj_iterate_impl
0x802d3f52 bpobj_iterate+15:  pop%rbp
0x802d3f53 bpobj_iterate+16:  retq

So quite obviously fbt can not really entry/return points for the clang 
function.

This is not a big problem on its own, of course, but here is a bad twist.
On the clang system:
$ ctfdump -f /boot/kernel/kernel | fgrep bpobj_iterate
  [8975] FUNC (bpobj_iterate) returns: 24 args: (2601, 4824, 34, 2221)
  [13093] FUNC (bpobj_iterate_nofree) returns: 24 args: (2601, 4824, 34, 2221)

Now that's the problem: fbt sees only bpobj_iterate_impl, but CTF data is
generated/present only for bpobj_iterate and bpobj_iterate_nofree.

On the gcc system:
ctfdump -f /boot/kernel/kernel | fgrep bpobj_iterate
  [323] FUNC (bpobj_iterate_impl) returns: 1 args: (5153, 5661, 6, 5078, 1350)
  [10439] FUNC (bpobj_iterate) returns: 1 args: (5153, 5661, 6, 5078)
  [14377] FUNC (bpobj_iterate_nofree) returns: 1 args: (5153, 5661, 6, 5078)

To summarize: I would be glad of either clang generated code was fbt-friendly
or if ctf information was generated for bpobj_iterate_impl.  Either is perfect
for me.

Now, I am not quite sure why ctfconvert skips bpobj_iterate_impl in the
clang-generated code.  Seems like some sort of a bug in ctfconvert.

-- 
Andriy Gapon
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org

Re: clang generated code sometimes confuses fbt

2013-03-02 Thread Andriy Gapon
on 02/03/2013 19:35 Andriy Gapon said the following:
 Now, I am not quite sure why ctfconvert skips bpobj_iterate_impl in the
 clang-generated code.  Seems like some sort of a bug in ctfconvert.

It seems that gcc and clang put different names for symbol of type FILE:
clang:
readelf -a -W /usr/obj/usr/src/sys/TRANT/bpobj.o| fgrep -w FILE
 1:  0 FILELOCAL  DEFAULT  ABS
/usr/src/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/bpobj.c

gcc:
readelf -a -W /usr/obj/usr/src/sys/ODYSSEY/bpobj.o| fgrep -w FILE
 1:  0 FILELOCAL  DEFAULT  ABS bpobj.c

ctfconvert seems to compare this value with bpobj.c and so in the clang case
it doesn't recognize the static symbols.

Does my analysis seem reasonable?
-- 
Andriy Gapon
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org

Re: clang generated code sometimes confuses fbt

2013-03-02 Thread Dimitry Andric

On 2013-03-02 18:52, Andriy Gapon wrote:

on 02/03/2013 19:35 Andriy Gapon said the following:

Now, I am not quite sure why ctfconvert skips bpobj_iterate_impl in the
clang-generated code.  Seems like some sort of a bug in ctfconvert.


It seems that gcc and clang put different names for symbol of type FILE:
clang:
readelf -a -W /usr/obj/usr/src/sys/TRANT/bpobj.o| fgrep -w FILE
  1:  0 FILELOCAL  DEFAULT  ABS
/usr/src/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/bpobj.c

gcc:
readelf -a -W /usr/obj/usr/src/sys/ODYSSEY/bpobj.o| fgrep -w FILE
  1:  0 FILELOCAL  DEFAULT  ABS bpobj.c

ctfconvert seems to compare this value with bpobj.c and so in the clang case
it doesn't recognize the static symbols.

Does my analysis seem reasonable?


Have you verified that ctfconvert does the right thing, if you modify
the FILE symbol to have just the filename?

Indeed, clang puts the original filename from the command line in the
.file directive, while gcc explicitly removes any directory names; see
contrib/gcc/toplev.c, around line 680:

  void
  output_file_directive (FILE *asm_file, const char *input_name)
  {
int len;
const char *na;

if (input_name == NULL)
  input_name = stdin;

len = strlen (input_name);
na = input_name + len;

/* NA gets INPUT_NAME sans directory names.  */
while (na  input_name)
  {
if (IS_DIR_SEPARATOR (na[-1]))
  break;
na--;
  }
...

That NA gets INPUT_NAME sans directory names comment was inserted by
rms in r279. :-)  So I guess this is the way gcc has done it from the
start, but there is no explanation as to why rms chose to remove those
directory names.  I do not see the problem, except maybe for having
reproducible builds?

-Dimitry
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


Re: clang generated code sometimes confuses fbt

2013-03-02 Thread Konstantin Belousov
On Sat, Mar 02, 2013 at 09:23:15PM +0100, Dimitry Andric wrote:
 On 2013-03-02 18:52, Andriy Gapon wrote:
  on 02/03/2013 19:35 Andriy Gapon said the following:
  Now, I am not quite sure why ctfconvert skips bpobj_iterate_impl in the
  clang-generated code.  Seems like some sort of a bug in ctfconvert.
 
  It seems that gcc and clang put different names for symbol of type FILE:
  clang:
  readelf -a -W /usr/obj/usr/src/sys/TRANT/bpobj.o| fgrep -w FILE
1:  0 FILELOCAL  DEFAULT  ABS
  /usr/src/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/bpobj.c
 
  gcc:
  readelf -a -W /usr/obj/usr/src/sys/ODYSSEY/bpobj.o| fgrep -w FILE
1:  0 FILELOCAL  DEFAULT  ABS bpobj.c
 
  ctfconvert seems to compare this value with bpobj.c and so in the clang 
  case
  it doesn't recognize the static symbols.
 
  Does my analysis seem reasonable?
 
 Have you verified that ctfconvert does the right thing, if you modify
 the FILE symbol to have just the filename?
 
 Indeed, clang puts the original filename from the command line in the
 .file directive, while gcc explicitly removes any directory names; see
 contrib/gcc/toplev.c, around line 680:
 
void
output_file_directive (FILE *asm_file, const char *input_name)
{
  int len;
  const char *na;
 
  if (input_name == NULL)
input_name = stdin;
 
  len = strlen (input_name);
  na = input_name + len;
 
  /* NA gets INPUT_NAME sans directory names.  */
  while (na  input_name)
{
   if (IS_DIR_SEPARATOR (na[-1]))
 break;
   na--;
}
 ...
 
 That NA gets INPUT_NAME sans directory names comment was inserted by
 rms in r279. :-)  So I guess this is the way gcc has done it from the
 start, but there is no explanation as to why rms chose to remove those
 directory names.  I do not see the problem, except maybe for having
 reproducible builds?

I seems that at least gdb also depends on the stripping the path for stabs
(which is not dwarf) debugging format interpretation. On the FreeBSD
system, do 'info', select 'stabs' - Stab Sections - Elf Linker
Relocation. The last paragraph documents the gdb requirements.

So it seems that stripped path in the STT_FILE is the common expectation.


pgpIIMEAO2ALX.pgp
Description: PGP signature