Commments on 2.1 draft 1 (A)

[davea at quasar dot engr dot sgi dot com (David B Anderson)]

The following are my comments on the April 5 2000 draft 1.

The document is quite nice, but there are a few things that, I think,
need attention (beyond Ron's TBD-marked areas).

I considered writing each of these as a separate email, but
that became rather a long list (would you want that many 
pieces of email at once?).

Some of these are simply matters of editorial taste
(so others will likely disagree).
Some are a bit stronger than just 'taste'.
Some are matters of correctness.

No doubt Ron is aware of some or all of these already, but
I wade in anyway.

===CONTENTS, page iv
I cannot find an entry here for "extended-section-offset"
(or whatever we finally call it).  See below where I
suggest this term instead of 64 bit safe.
But my point here is that nothing about this shows in the TOC.
(see later for more on this, sec 7.4)


====FOREWORD page viii
"but has additional links"
is some kind of jargon. I'm not sure exactly what is meant by 'links'.
Lets not use such a term here (it's not html).

==== 2.4 dwarf expressions, page 11
"In DWARF Version 2, all DWARF"  should be
"In DWARF Version 2.0.0, all DWARF" I think.

==== 2.4.2.5 ControlFlow Operations  page 18

In the OP_call descr:
"of a DIE in the current unit" should be
"of a DIE in the current compilation unit"  I think

"Execution of the DW_AT_location attribute" should be
"Execution of the DWARF expression of the  DW_AT_location attribute" 

and later in that para. 'called attribute' should be
	'called DWARF expression' in two places, I suggest.


==== 2.13 Declaration coordinates page 26, also page 79

"statement information table" is  mentioned a couple times, but
that phrase is not used at the definition of the line table.
That is, there is a lack of consistency here in the
way the line/statement information is mentioned. This
lack of consistency is not a good thing, IMO.

This consistency problem is not new AFAIK. But
removing the inconsistencies and using a single terminology
would be a good idea, I think.


==== 3.1 Compilation Unit Entries page 30
The description DW_AT_base_types is still not clear.

It's useful for interpreting a conversion to a base type 
when that base type is not mentioned in a current compilation
unit, as then there is no reference chain for a debugger to follow,
so a commonized list of base types  off in some
other compilation unit might help.

It's also not clear (any more than in the original document)
what DIE type this might point to. CU die? Something else?
Did we ever decide this for the original document?
Am I just missing something here?


=== 5.4 array type entries, page 48
It would be good here to make a forward reference to
5.13, Dynamic Type Properties, and DW_AT_data_location.
Not to repeat anything here from 5.13, 
just as a hint there is more available that is described later.



====5.4 array type entries, page 49.

"Note that if the size"...
is in italics and the placement makes the following read better, IMO:
"If the size"...

The "Note that" here are just noise words here and add nothing. IMO.

(Not condemning "Note" in general...)


==== 5.5.4 Structure Data Member Entries  p 51

It seems odd to me that 5.5.2 specifically mentions
DW_AT_accessibility, whereas 5.5.4 does not.  5.5.4 should
mention it too, as it is very meaningful there for C++.


==== 5.11 pointer to member type entries p 60
"Finally, the pointer to member entry has a DW_AT_use_location
attribute whose value is a location description that computes
the address of the member of the class or structure
to which the pointer to member entry can point".

The above makes no sense. Well, less than it should. I think
the intended meaning is expressed by:

"Finally, the pointer to member entry has a DW_AT_use_location
attribute whose value is a location description that computes
the address of the member of the class or structure
to which the pointer to member entry points (the value
is meaningless if the pointer-to-member does not currently
point at anything).".

But maybe I'm just confused???


====5.13.2 allocation and association status  pg 62

"may then optionally be" should be
"may then be"  I think.

====6.1.1 lookup by name pge 63

"the names of global objects whose definitions or declarations"
is misleading if you think of 'objects' as in the C standard or
C++ standard.


"the names of global objects or functions whose definitions or declarations"


====6.1.1 lookup by name pge 64
"the name of a static data member or function member of a C++ structure" 
should be

"the name of a static-data-member or function-member of a C++ structure"  

as otherwise it's unclear if 'static' should apply
to the 'function' part.


I worry (as Ron did) about the use of 'declaration' in 6.1.1
and do not understand why a declaration would usefully appear here.

Is it just Ron and I?


==== 6.2.4 statement program prologe page 69

This is (I think) the
first appearance of '64-bit safe DWARF' and, like
Mike, I think this is the wrong phrase to use.
It's not accurate.

After pondering for a few minutes, I think that
  "extended-section-offset DWARF"
would be a better description.
(not perfect, but better)

The phrase appears many places of course.


====7.5.4 attribute encodings page 94


See the paragraph beginning "There are two types of reference."


"There are five forms for this type of reference: one, two, foru, and 
eight byte offsets (respectively DW_FORM_ref1, DW_FORM_ref2,
DW_FORM_ref4, and DW_FORM_ref8). There is also an unsigned
variable length offset encoded form that
uses LEB128 numbers (DW_FORM_ref_udata).
This offset is relocated (non-relocatable) in all forms of object file.

The above has various problems (that last sentence makes no 
sense to me... Ah. Now I perhaps sense its meaning.)

Note that some of the problems (lack of parallelism, for example)
are in the original 2.0.0 text as well.

I would suggest, instead.

"There are five forms for this type of reference.
DW_FORM_ref1, DW_FORM_ref2, DW_FORM_ref4, DW_FORM_ref8,
and DW_FORM_ref_udata.  Because the reference is
within  a single compilation unit no relocation
of the value is required."


====7.5.4 attribute encodings page 94
The paragraph beginning just after the above, 
"The second type of reference"

mentions "an offset from the beginning of the .debug_info section".
I was wondering if (this is a general comment) all references
to 'offset' should be restated as
	offset or extended-section-offset

Actually it might be better to have a definition of terms,
and define offset briefly there as meaning both the
basic 4 byte offset and extended-section-offset
as appropriate for a given compilation unit.


====7.5.4 attribute encodings page 94

Mentions 4-byte offset. One of many places in the document...

====Figure 32, page 116.

Omits DW_LNS_set_prologue_end DW_LNS_set_epilogue_begin.


====7.2.4 Dependencies, page 119

"In target architectures with 64-bit addresses, the
assembler and linker must similarly handle 8-byte references
at arbitrary alignments."

Might be more complete as:

"In target architectures with 64-bit addresses
the
assembler and linker must similarly handle 8-byte references
at arbitrary alignments (extended-section-offsets also require
8 byte reference relocations)."



====Appendix 2, Organization of Debugging Information page 143

This draft printout of this section is unfortunately
unintelligible.  It needs to get back onto one page,
properly laid out as per the v2.0.0 document.

====Appendix 3, Statement Program Examples, page 147.

The print here is too big. Unless one gets the whole
works onto one page the result is unreadable, as
one must look back and forth many times in studying
the example.  Get it all on one page if at all possible.
Or at least get the beginning plus the first encoding
on one page. But preferably all.



==== Appendix 6 Aggregate examples, page 155

First paragraph, f90 example:

"necessarily longer (includes) that of the raw data"

would really be, IMO


"necessarily longer than and includes that of the raw data,"


Second paragraph:

"2) there doesn't even need"

might be better as

"2) there doesn't need"


==== Appendix 6 Aggregate examples, page 156

Just before the paragraph beginning "The F90 derived type array_ptr"
I suggest adding a paragraph:

"If an object has a descriptor, then the dwarf type for the object
will have the DW_AT_data_location attribute. If an object
does not have a descriptor, then the dwarf type for the object
will not have the DW_AT_data_location attribute."

This is to reinforce what I think Ron means...

==== Appendix 6 Aggregate examples, page 156

Change
	"fixed compiletime known size"
to
	"fixed compile time size"
, IMO.

Note that the "desc<1>" sentence that this refers to is
one that I had to stare at quite a while before I began
to understand it.  It could just be me or it could be
that this needs some rephrasing or elaboration?


==== Appendix 6 Aggregate examples, page 157

In several places there is
	DW_OP_push_object_address
	DW_OP_deref
	DW_OP_lit0
	DW_OP_ne
I think this is incorrect.
I think these all should be
	DW_OP_push_object_address
	DW_OP_deref
	DW_OP_lit1     !value corresponding to bit position
		       ! of the associated flag, to mask
		       ! so we only test the 'associated' bit.
	DW_OP_and
	DW_OP_lit0
	DW_OP_ne

	


==== Appendix 6 Aggregate examples, page 157, etc

DW_OP_add appears a few times. Should be DW_OP_plus


==== Appendix 6 Aggregate examples, page 157, etc

I think the TAGS should be slightly indented where
appropriate to indicate child (vs sibling) relationship.
The existing layout obscures this essential feature of
DW_TAG_subrange_type, for example.

==== Appendix 6 Aggregate examples, page 160

"This is all just like step 2), so the details are omited.
Suffice it to note that the object address of interest here
is the address that resulted from step 4."

I believe this is correct, but I think it should be rephrased.
(aside from the misspelling of omited[sic]).

It does not emphasize enough the key things of interest.
a) that because the type ($1) has DW_AT_data_location 
the object address we get from step 4 is a descriptor object.
This is a strong statement about how the code generator works
(this is also a requirement for correct code generation,
I believe, but it is nontheless a strong statement about
the generated code).

b) The DW_OP_push_object_address in 1$ and 2$ refer to this object
which is, per the definition, a descriptor.

In other words, I'm asking for a more thorough explanation
here, to forestall confusion.

=== extended-section-offset
or whatever we call it.

Now I cannot find the text describing this (just mentions
of the 64bit-safe term (which, as I said, I don't like))..

Ah. I found this searching for 'safe' and it's TBD.
Section 7.4.    But the name does not relate to
the other terminolgy, so I think 7.4 should be
called extended-dwarf-offset or whatever
we finally call this, so the TOC has something that
relates directly to the text.

We need to make clear in 7.4, that
this is  necessary to allow dwarf sections to grow
over 4GByte, and that it allows mixing of 32bit offsets
(from some compilation units) 
with extended-section-offsets (from other compilation units).
But that the only guaranteed way of being
able to create and link sections over
4GByte in size
is for all compilation units in that section of that
object to have extended-section-offsets.


------davea@sgi.com