Re: [PATCH v4] covoar: Handle periods in symbols from objdump
this one looks fine to me.
On Fri, Mar 26, 2021 at 3:21 PM Alex White wrote:
>
> ping
>
> > -Original Message-
> > From: Alex White
> > Sent: Friday, March 19, 2021 2:10 PM
> > To: devel@rtems.org
> > Cc: Alex White
> > Subject: [PATCH v4] covoar: Handle periods in symbols from objdump
> >
> > Occasionally the compiler will generate symbols that look similar to symbols
> > defined in RTEMS code except that they contain some suffix.
> > These symbol suffixes are only found in the ELF symbol table; the symbols
> > appear to be normal in the DWARF info. This appears to be happening on all
> > architectures.
> >
> > For example, the function _Message_queue_Create from rtems appears as
> > "_Message_queue_Create.part.0". Other suffixes include ".isra.0",
> > ".constprop.0", and ".0".
> >
> > This looks to be related to compiler optimizations. Symbols with suffixes
> > were being treated as unique. For our purposes, they should be mapped to
> > the equivalent symbols in the DWARF info. This has been fixed.
> > ---
> > tester/covoar/ExecutableInfo.cc | 3 +--
> > tester/covoar/ObjdumpProcessor.cc | 15 +++
> > tester/covoar/SymbolTable.cc | 12 +---
> > 3 files changed, 25 insertions(+), 5 deletions(-)
> >
> > diff --git a/tester/covoar/ExecutableInfo.cc
> > b/tester/covoar/ExecutableInfo.cc index c996d75..1e14721 100644
> > --- a/tester/covoar/ExecutableInfo.cc
> > +++ b/tester/covoar/ExecutableInfo.cc
> > @@ -118,8 +118,7 @@ namespace Coverage {
> > // Obtain the coverage map containing the specified address.
> > itsSymbol = theSymbolTable.getSymbol( address );
> > if (itsSymbol != "") {
> > - it = coverageMaps.find( itsSymbol );
> > - aCoverageMap = (*it).second;
> > + aCoverageMap = (itsSymbol);
> > }
> >
> > return aCoverageMap;
> > diff --git a/tester/covoar/ObjdumpProcessor.cc
> > b/tester/covoar/ObjdumpProcessor.cc
> > index 0647ff9..e19fa92 100644
> > --- a/tester/covoar/ObjdumpProcessor.cc
> > +++ b/tester/covoar/ObjdumpProcessor.cc
> > @@ -417,6 +417,21 @@ namespace Coverage {
> > processSymbol = false;
> > theInstructions.clear();
> >
> > +// Look for a '.' character and strip everything after it.
> > +// There is a chance that the compiler splits function bodies to
> > improve
> > +// inlining. If there exists some inlinable function that contains
> > a
> > +// branch where one path is more expensive and less likely to be
> > taken
> > +// than the other, inlining only the branch instruction and the
> > less
> > +// expensive path results in smaller code size while preserving
> > most of
> > +// the performance improvement.
> > +// When this happens, the compiler will generate a function with a
> > +// ".part.n" suffix. For our purposes, this generated function
> > part is
> > +// equivalent to the original function and should be treated as
> > such.
> > +char *periodIndex = strstr(symbol, ".");
> > +if (periodIndex != NULL) {
> > + *periodIndex = 0;
> > +}
> > +
> > // See if the new symbol is one that we care about.
> > if (SymbolsToAnalyze->isDesired( symbol )) {
> >currentSymbol = symbol;
> > diff --git a/tester/covoar/SymbolTable.cc b/tester/covoar/SymbolTable.cc
> > index 53bc8af..00062cc 100644
> > --- a/tester/covoar/SymbolTable.cc
> > +++ b/tester/covoar/SymbolTable.cc
> > @@ -46,12 +46,18 @@ namespace Coverage {
> > symbolData.startingAddress = start;
> > symbolData.length = length;
> >
> > -if ( info[ symbol ].empty() == false ) {
> > - if ( info[ symbol ].front().length != length ) {
> > +for (auto& symData : info[ symbol ]) {
> > + // The starting address could differ since we strip any suffixes
> > beginning
> > + // with a '.'
> > + if (symData.startingAddress != start) {
> > +continue;
> > + }
> > +
> > + if (symData.length != length) {
> > std::ostringstream what;
> > what << "Different lengths for the symbol "
> > << symbol
> > - << " (" << info[ symbol ].front().length
> > + << " (" << symData.length
> > << " and " << length
> > << ")";
> > throw rld::error( what, "SymbolTable::addSymbol" );
> > --
> > 2.27.0
>
> ___
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
RE: [PATCH v4] covoar: Handle periods in symbols from objdump
ping
> -Original Message-
> From: Alex White
> Sent: Friday, March 19, 2021 2:10 PM
> To: devel@rtems.org
> Cc: Alex White
> Subject: [PATCH v4] covoar: Handle periods in symbols from objdump
>
> Occasionally the compiler will generate symbols that look similar to symbols
> defined in RTEMS code except that they contain some suffix.
> These symbol suffixes are only found in the ELF symbol table; the symbols
> appear to be normal in the DWARF info. This appears to be happening on all
> architectures.
>
> For example, the function _Message_queue_Create from rtems appears as
> "_Message_queue_Create.part.0". Other suffixes include ".isra.0",
> ".constprop.0", and ".0".
>
> This looks to be related to compiler optimizations. Symbols with suffixes
> were being treated as unique. For our purposes, they should be mapped to
> the equivalent symbols in the DWARF info. This has been fixed.
> ---
> tester/covoar/ExecutableInfo.cc | 3 +--
> tester/covoar/ObjdumpProcessor.cc | 15 +++
> tester/covoar/SymbolTable.cc | 12 +---
> 3 files changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/tester/covoar/ExecutableInfo.cc
> b/tester/covoar/ExecutableInfo.cc index c996d75..1e14721 100644
> --- a/tester/covoar/ExecutableInfo.cc
> +++ b/tester/covoar/ExecutableInfo.cc
> @@ -118,8 +118,7 @@ namespace Coverage {
> // Obtain the coverage map containing the specified address.
> itsSymbol = theSymbolTable.getSymbol( address );
> if (itsSymbol != "") {
> - it = coverageMaps.find( itsSymbol );
> - aCoverageMap = (*it).second;
> + aCoverageMap = (itsSymbol);
> }
>
> return aCoverageMap;
> diff --git a/tester/covoar/ObjdumpProcessor.cc
> b/tester/covoar/ObjdumpProcessor.cc
> index 0647ff9..e19fa92 100644
> --- a/tester/covoar/ObjdumpProcessor.cc
> +++ b/tester/covoar/ObjdumpProcessor.cc
> @@ -417,6 +417,21 @@ namespace Coverage {
> processSymbol = false;
> theInstructions.clear();
>
> +// Look for a '.' character and strip everything after it.
> +// There is a chance that the compiler splits function bodies to
> improve
> +// inlining. If there exists some inlinable function that contains a
> +// branch where one path is more expensive and less likely to be
> taken
> +// than the other, inlining only the branch instruction and the less
> +// expensive path results in smaller code size while preserving most
> of
> +// the performance improvement.
> +// When this happens, the compiler will generate a function with a
> +// ".part.n" suffix. For our purposes, this generated function part
> is
> +// equivalent to the original function and should be treated as such.
> +char *periodIndex = strstr(symbol, ".");
> +if (periodIndex != NULL) {
> + *periodIndex = 0;
> +}
> +
> // See if the new symbol is one that we care about.
> if (SymbolsToAnalyze->isDesired( symbol )) {
>currentSymbol = symbol;
> diff --git a/tester/covoar/SymbolTable.cc b/tester/covoar/SymbolTable.cc
> index 53bc8af..00062cc 100644
> --- a/tester/covoar/SymbolTable.cc
> +++ b/tester/covoar/SymbolTable.cc
> @@ -46,12 +46,18 @@ namespace Coverage {
> symbolData.startingAddress = start;
> symbolData.length = length;
>
> -if ( info[ symbol ].empty() == false ) {
> - if ( info[ symbol ].front().length != length ) {
> +for (auto& symData : info[ symbol ]) {
> + // The starting address could differ since we strip any suffixes
> beginning
> + // with a '.'
> + if (symData.startingAddress != start) {
> +continue;
> + }
> +
> + if (symData.length != length) {
> std::ostringstream what;
> what << "Different lengths for the symbol "
> << symbol
> - << " (" << info[ symbol ].front().length
> + << " (" << symData.length
> << " and " << length
> << ")";
> throw rld::error( what, "SymbolTable::addSymbol" );
> --
> 2.27.0
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
[PATCH v4] covoar: Handle periods in symbols from objdump
Occasionally the compiler will generate symbols that look similar to
symbols defined in RTEMS code except that they contain some suffix.
These symbol suffixes are only found in the ELF symbol table; the
symbols appear to be normal in the DWARF info. This appears to be
happening on all architectures.
For example, the function _Message_queue_Create from rtems appears as
"_Message_queue_Create.part.0". Other suffixes include ".isra.0",
".constprop.0", and ".0".
This looks to be related to compiler optimizations. Symbols with
suffixes were being treated as unique. For our purposes, they should be
mapped to the equivalent symbols in the DWARF info. This has been
fixed.
---
tester/covoar/ExecutableInfo.cc | 3 +--
tester/covoar/ObjdumpProcessor.cc | 15 +++
tester/covoar/SymbolTable.cc | 12 +---
3 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/tester/covoar/ExecutableInfo.cc b/tester/covoar/ExecutableInfo.cc
index c996d75..1e14721 100644
--- a/tester/covoar/ExecutableInfo.cc
+++ b/tester/covoar/ExecutableInfo.cc
@@ -118,8 +118,7 @@ namespace Coverage {
// Obtain the coverage map containing the specified address.
itsSymbol = theSymbolTable.getSymbol( address );
if (itsSymbol != "") {
- it = coverageMaps.find( itsSymbol );
- aCoverageMap = (*it).second;
+ aCoverageMap = (itsSymbol);
}
return aCoverageMap;
diff --git a/tester/covoar/ObjdumpProcessor.cc
b/tester/covoar/ObjdumpProcessor.cc
index 0647ff9..e19fa92 100644
--- a/tester/covoar/ObjdumpProcessor.cc
+++ b/tester/covoar/ObjdumpProcessor.cc
@@ -417,6 +417,21 @@ namespace Coverage {
processSymbol = false;
theInstructions.clear();
+// Look for a '.' character and strip everything after it.
+// There is a chance that the compiler splits function bodies to
improve
+// inlining. If there exists some inlinable function that contains a
+// branch where one path is more expensive and less likely to be taken
+// than the other, inlining only the branch instruction and the less
+// expensive path results in smaller code size while preserving most of
+// the performance improvement.
+// When this happens, the compiler will generate a function with a
+// ".part.n" suffix. For our purposes, this generated function part is
+// equivalent to the original function and should be treated as such.
+char *periodIndex = strstr(symbol, ".");
+if (periodIndex != NULL) {
+ *periodIndex = 0;
+}
+
// See if the new symbol is one that we care about.
if (SymbolsToAnalyze->isDesired( symbol )) {
currentSymbol = symbol;
diff --git a/tester/covoar/SymbolTable.cc b/tester/covoar/SymbolTable.cc
index 53bc8af..00062cc 100644
--- a/tester/covoar/SymbolTable.cc
+++ b/tester/covoar/SymbolTable.cc
@@ -46,12 +46,18 @@ namespace Coverage {
symbolData.startingAddress = start;
symbolData.length = length;
-if ( info[ symbol ].empty() == false ) {
- if ( info[ symbol ].front().length != length ) {
+for (auto& symData : info[ symbol ]) {
+ // The starting address could differ since we strip any suffixes
beginning
+ // with a '.'
+ if (symData.startingAddress != start) {
+continue;
+ }
+
+ if (symData.length != length) {
std::ostringstream what;
what << "Different lengths for the symbol "
<< symbol
- << " (" << info[ symbol ].front().length
+ << " (" << symData.length
<< " and " << length
<< ")";
throw rld::error( what, "SymbolTable::addSymbol" );
--
2.27.0
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
