Fix multiple issues with ddrgen #2271

keithc-ca · 2018-01-29T17:02:27Z

This is in support of eclipse-openj9/openj9#378.

Fixes & improvements

blacklist pattern matching
close list files
avoid typedef cycles
write proper enum size to blob
handle CR/LF line-ends reading a list files
pass const references
favour constructor use over assignment
formatting
improve safety
mark constructors explicit as appropriate
improve DEBUGPRINTF and ERRMSG macros and use consistent format
remove redundant or unnecessary #includes
add constructor for Dwarf_Attribute_s
add Dwarf_Attribute_s._flag
add dwarf_formflag() for OSX
omit static fields from expected superset.out
improve handling of vtable pointers
improve 'getmacros' script (and don't let it change timestamps)
use 'find' to avoid scanning unintersting files
pull regex definitions out of the inner loop
fix references to ddr/tools/getmacros
improve test code
add an example of map_to_type= without following whitespace
improve namespace handling
improve getmacros script
replace use of 'awk' with 'sed'
fix @macro emissions
tighten API of ddrgen
fix validation that macro values are constant
streamline MacroTool
fix constant evaluation (including numeric prefixes like '0x')
fix problem reading macrolist
use const types where appropriate
use canonical type expressions
make as many *.i files as possible
be careful constructing name to type map for macros
implicitly blacklist illegal type names
remove overloading in TypeVisitor
allow writing of blob and superset independently
simplify blacklist handling
make reading blacklist more robust
fix override support
fix handling of #undef by getmacros
ignore linkage_name DWARF attribute
add ClassType::_isComplete
construct all fields explicitly
relocate constants to J9HashtableConstants
repeat structure for typedef with distinct name

ddrgen

consider macros in ClassType
fix Type::renameFieldsAndMacros
don't require values for enum literals
don't trim trailing *[] from override value
generalize support for opaque types
include typedef with distinct name
new strategy for eliminating duplicate types
remove special handling for CLimits
remove unused _offset field in Modifiers
reset modifiers with field type override

Blob generation

don't duplicate fields in blob
extract common code
propagate failure to parent process

getmacros

fix handling of ddr_options: valuesandbuildflags
improve include guard detection
fix constant handling

would previously map to the following
(independent of whether A was defined or not)

@MACRO_A_DEFINED A_DEFINED 1
@MACRO_A_DEFINED A_DEFINED 0

@macro

Fixes & improvements * blacklist pattern matching * close list files * avoid typedef cycles * write proper enum size to blob * handle CR/LF line-ends reading a list files * pass const references * favour constructor use over assignment * formatting * improve safety * mark constructors explicit as appropriate * improve DEBUGPRINTF and ERRMSG macros and use consistent format * remove redundant or unnecessary #includes * add constructor for Dwarf_Attribute_s * add Dwarf_Attribute_s._flag * add dwarf_formflag() for OSX * omit static fields from expected superset.out * improve handling of vtable pointers * improve 'getmacros' script (and don't let it change timestamps) * use 'find' to avoid scanning unintersting files * pull regex definitions out of the inner loop * fix references to ddr/tools/getmacros * improve test code * add an example of map_to_type= without following whitespace * improve namespace handling * improve getmacros script * replace use of 'awk' with 'sed' * fix @macro emissions * tighten API of ddrgen * fix validation that macro values are constant * streamline MacroTool * fix constant evaluation (including numeric prefixes like '0x') * fix problem reading macrolist * use const types where appropriate * use canonical type expressions * make as many *.i files as possible * be careful constructing name to type map for macros * implicitly blacklist illegal type names * remove overloading in TypeVisitor * allow writing of blob and superset independently * simplify blacklist handling * make reading blacklist more robust * fix override support * fix handling of #undef by getmacros * ignore linkage_name DWARF attribute * add ClassType::_isComplete * construct all fields explicitly * relocate constants to J9HashtableConstants * repeat structure for typedef with distinct name ddrgen * consider macros in ClassType * fix Type::renameFieldsAndMacros * don't require values for enum literals * don't trim trailing *[] from override value * generalize support for opaque types * include typedef with distinct name * new strategy for eliminating duplicate types * remove special handling for CLimits * remove unused _offset field in Modifiers * reset modifiers with field type override Blob generation * don't duplicate fields in blob * extract common code * propagate failure to parent process getmacros * fix handling of ddr_options: valuesandbuildflags * improve include guard detection * fix constant handling would previously map to the following (independent of whether A was defined or not) ``` @MACRO_A_DEFINED A_DEFINED 1 @MACRO_A_DEFINED A_DEFINED 0 ``` Signed-off-by: Keith W. Campbell <[email protected]>

Signed-off-by: Keith W. Campbell <[email protected]>

charliegracie · 2018-01-30T16:26:21Z

@genie-omr build zos zlinux plinux

charliegracie · 2018-01-30T19:27:15Z

ddr/lib/ddr-ir/Modifiers.cpp

-		for (size_t i = 0; i < getArrayDimensions(); i++) {
-			s += "[]";
-		}
+	for (size_t i = getArrayDimensions(); i != 0; i -= 1) {


I guess getArrayDimensions() returns 0 if isArray() returns false?

charliegracie · 2018-01-30T19:29:41Z

ddr/lib/ddr-scanner/Scanner.cpp

+{
+	bool match = false; // patterns.find(candidate) != patterns.end();
+
+	if (!match) {


Can we get rid of match?

keithc-ca · 2018-02-05T15:01:30Z

@charliegracie Any more feedback or questions?

* remove redundant local variable Signed-off-by: Keith W. Campbell <[email protected]>

Changes requested during review of #2271

keithc-ca added 2 commits January 29, 2018 11:46

Include omrcomp.h to get int32_t, etc.

61ade0e

Signed-off-by: Keith W. Campbell <[email protected]>

keithc-ca force-pushed the ddr branch from be040b7 to 61ade0e Compare January 29, 2018 18:41

keithc-ca added 2 commits January 29, 2018 14:17

Define DW_AT_external

9bfe50e

Signed-off-by: Keith W. Campbell <[email protected]>

Define a replacement for strtoll() for Visual Studio

672762e

Signed-off-by: Keith W. Campbell <[email protected]>

mstoodle added comp:diagnostic tooling labels Jan 29, 2018

charliegracie reviewed Jan 30, 2018

View reviewed changes

charliegracie self-assigned this Feb 12, 2018

charliegracie approved these changes Feb 12, 2018

View reviewed changes

charliegracie merged commit 2b5d8e1 into eclipse-omr:master Feb 12, 2018

keithc-ca deleted the ddr branch February 12, 2018 14:12

keithc-ca added a commit to keithc-ca/omr that referenced this pull request Feb 12, 2018

Changes requested during review of eclipse-omr#2271

d909e48

* remove redundant local variable Signed-off-by: Keith W. Campbell <[email protected]>

charliegracie added a commit that referenced this pull request Feb 13, 2018

Merge pull request #2308 from keithc-ca/ddr-review

eb19669

Changes requested during review of #2271

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix multiple issues with ddrgen #2271

Fix multiple issues with ddrgen #2271

keithc-ca commented Jan 29, 2018

charliegracie commented Jan 30, 2018

charliegracie Jan 30, 2018

keithc-ca Jan 30, 2018

charliegracie Jan 30, 2018

keithc-ca Jan 30, 2018

keithc-ca commented Feb 5, 2018

Fix multiple issues with ddrgen #2271

Fix multiple issues with ddrgen #2271

Conversation

keithc-ca commented Jan 29, 2018

charliegracie commented Jan 30, 2018

charliegracie Jan 30, 2018

Choose a reason for hiding this comment

keithc-ca Jan 30, 2018

Choose a reason for hiding this comment

charliegracie Jan 30, 2018

Choose a reason for hiding this comment

keithc-ca Jan 30, 2018

Choose a reason for hiding this comment

keithc-ca commented Feb 5, 2018