[PATCH] D30448: [DebugInfo] Show implicit_const values when dumping .debug_info section

[Victor Leschuk via Phabricator via llvm-commits]

vleschuk marked an inline comment as done.
vleschuk added inline comments.


================
Comment at: lib/DebugInfo/DWARF/DWARFDie.cpp:94-95
   
-  if (!formValue.extractValue(U->getDebugInfoExtractor(), OffsetPtr, U))
+  if (AS.isImplicitConst())
+    formValue.setSValue(*AS.ByteSizeOrValue);
+  else if (!formValue.extractValue(U->getDebugInfoExtractor(), OffsetPtr, U))
----------------
dblaikie wrote:
> I wouldn't expect to see a special case here. Is it necessary? I'd expect extractValue to do the work necessary for whatever the attribute kind is (as it already does for form_present, I assume).
> 
> I guess the issue is that DWARFFormValue only holds the form enum, not the rest - I think that probably should be changed so that extractValue can still behave uniformly across all forms.
> 
> So DWARFFormValue should take the AttributeSpec... oh, but that has the attribute in it.
> 
> Maybe what's needed is a thing that represents the form +ByteSizeOrValue. Maybe it could be called "FormSpec" to match "AttributeSpec"? Not sure.
> I guess the issue is that DWARFFormValue only holds the form enum, not the rest - I think that probably should be changed so that extractValue can still behave uniformly across all forms.

Actually DWARFFormValue holds not only enum but actual value too:

```
  struct ValueType {                                                            
    ValueType() {                                                               
      uval = 0;                                                                 
    }                                                                           
                                                                                
    union {                                                                     
      uint64_t uval;                                                            
      int64_t sval;                                                             
      const char* cstr;                                                         
    };                                                                          
    const uint8_t* data = nullptr;                                              
  };                                                                            
                                                                                
  dwarf::Form Form; // Form for this value.                                     
  ValueType Value; // Contains all data for the form.                             struct ValueType {                                                            
    ValueType() {                                                               
      uval = 0;                                                                 
    }                                                                           
                                                                                
    union {                                                                     
      uint64_t uval;                                                            
      int64_t sval;                                                             
      const char* cstr;                                                         
    };                                                                          
    const uint8_t* data = nullptr;                                              
  };                                                                            
                                                                                
  dwarf::Form Form; // Form for this value.                                     
  ValueType Value; // Contains all data for the form.                           
  struct ValueType {                                                            
    ValueType() {                                                               
      uval = 0;                                                                 
    }                                                                           
                                                                                
    union {                                                                     
      uint64_t uval;                                                            
      int64_t sval;                                                             
      const char* cstr;                                                         
    };                                                                          
    const uint8_t* data = nullptr;                                              
  };                                                                            
                                                                                
  dwarf::Form Form; // Form for this value.                                     
  ValueType Value; // Contains all data for the form.                           
```

What extractValue() does - it actually fills the ValueType field of DWARFFormValue, for flag_present it just sets Value to "true" and doesn't need additional data. To fill in implicit_const from inside of extractValue() we need to pass the actual value to it and than assign it from within:

```
formValue.extractValue(/* ... */, int64_t actualValue);
// ... 
bool DWARFFormValue::extractValue(/* ... */, int64_t actualValue) {
  // ....
  case DW_FORM_implicit_const:
  Value.sdata = actualValue;
  break;
  // ... 
```
That looks a little bit weird for me, as lots of useless actions are performed like passing data back and forth. I think having exceptional case for implicit_const in upper stack frame is better way here.


https://reviews.llvm.org/D30448