Re: [patch] PR symtab/13277: Resolving opaque structures in ICC generated binaries.

[John Steele Scott <toojays@toojays.net>]

Jan,

Thank you for continuing to work with me on this.

On 19/05/12 00:16, Jan Kratochvil wrote:
> Hi John,
>
> On Mon, 14 May 2012 15:51:20 +0200, John Steele Scott wrote:
>> Are the existing tests already known to work with ICC?
> I do not remember trying it.  I assume many tests will FAIL.  But we do not
> want to diff gcc results vs. icc results but only icc results before the patch
> vs. icc results after the patch.

I bit the bullet and installed icc on my local machine. It's quite painful due to (I think) a round-trip to the licensing server for each compilation unit. Anyway, I was able to run the testsuite. I run "make check" with  RUNTESTFLAGS="CC_FOR_TARGET=icc CFLAGS_FOR_TARGET='-debug extended' CXX_FOR_TARGET=icpc CXXFLAGS_FOR_TARGET='-debug extended'".

I saw later your wiki page at http://sourceware.org/gdb/wiki/Running%20the%20test%20suite%20with%20a%20non-GCC%20compiler. It seems icc does now understand --print-multi-lib.

> With my icc test there is difference between the types for v and w:
> 	struct s;
> 	extern struct s v;
> 	struct w {} w;
>
> DW_AT_producer    : Intel(R) C Intel(R) 64 Compiler XE for applications running on Intel(R) 64, Version 12.1.4.319 Build 20120410 Fixes SameLinkageName MemberPointers
>
>  <1><ed>: Abbrev Number: 4 (DW_TAG_variable)
>     <f2>   DW_AT_name        : v
>     <f4>   DW_AT_type        : <0xfa>
> [shortened]
>  <1><fa>: Abbrev Number: 5 (DW_TAG_structure_type)
>     <fb>   DW_AT_decl_line   : 1
>     <fc>   DW_AT_decl_column : 8
>     <fd>   DW_AT_decl_file   : 1
>     <fe>   DW_AT_accessibility: 1       (public)
>     <ff>   DW_AT_byte_size   : 0
>     <100>   DW_AT_name        : s
>  <1><102>: Abbrev Number: 6 (DW_TAG_variable)
>     <107>   DW_AT_name        : w
>     <109>   DW_AT_type        : <0x118>
> [shortened]
>  <1><118>: Abbrev Number: 5 (DW_TAG_structure_type)
>     <119>   DW_AT_decl_line   : 4
>     <11a>   DW_AT_decl_column : 8
>     <11b>   DW_AT_decl_file   : 1
>     <11c>   DW_AT_accessibility: 1      (public)
>     <11d>   DW_AT_byte_size   : 0
>     <11e>   DW_AT_name        : w
>
> This means icc does must not use TYPE_STUB_SUPPORTED.  Just one needs to fix
> producer_is_icc so that it gets processed only once and cached via
> 'checked_producer' as otherwise it may needlessly slow down reading DWARF.

If I understand you correctly, the problem with my previous approach is that it essentially ignores empty structs. And indeed now that I run the testsuite with ICC, it shows failures in gdb.base/nofield.exp. The approach you suggested passes that test.

However, this brings us back to the problem I wrote about last October (see http://article.gmane.org/gmane.comp.gdb.patches/69651): the zero-size structures are cluttering up the psymbols and basic_lookup_transparent_type will only do one psymbol->symbol expansion per call. For a frequently referenced opaque type in a non-trivial program, initially "ptype" will show it as "no data fields". But if I repeatedly invoke ptype, eventually it resolves the type correctly. Of course this does not happen with my trivial testcase, and I'm unsure how I would expand that to expose this issue.

Can you suggest a way to fix this, without ignoring zero-size structs?

If I have to chose between showing empty structs, and correctly resolving non-empty structs, I would prefer to resolve the non-empty ones, they are much more interesting. :)

On the question of caching the producer info in the dwarf2_cu; I propose to extract out the second half of producer_is_gxx_lt_4_6 into a new check_producer function which will set cu->producer_is_gxx_lt_4_6 and cu->producer_is_icc as appropriate (and then set cu->checked_producer). producer_is_gxx_lt_4_6 and producer_is_icc will call check_producer if cu->checked_producer is not set. Sound okay?

Thanks,

John