[PATCH] covoar: Add option to create named objdumps

Gedare Bloom gedare at rtems.org
Mon Mar 29 20:56:52 UTC 2021


On Fri, Mar 12, 2021 at 10:07 AM Alex White <alex.white at oarcorp.com> wrote:
>
> This adds a new behavior to the -d option which allows the creation of
> named objdump outputs in the /tmp directory. This allows the outputs to
> be reused on subsequent runs of covoar rather than running objdump
> again.
This seems to be a hackish way to optimize a behavior. Do you guys
have a plan to improve this caching mechanism?

> ---
>  tester/covoar/ObjdumpProcessor.cc | 22 +++++++++++++---------
>  tester/covoar/ObjdumpProcessor.h  |  6 ++++--
>  tester/covoar/app_common.cc       | 10 ++++++++++
>  tester/covoar/app_common.h        |  1 +
>  tester/covoar/covoar.cc           | 23 +++++++++++++++++++----
>  5 files changed, 47 insertions(+), 15 deletions(-)
>
> diff --git a/tester/covoar/ObjdumpProcessor.cc b/tester/covoar/ObjdumpProcessor.cc
> index 9ef2390..fb23ca6 100644
> --- a/tester/covoar/ObjdumpProcessor.cc
> +++ b/tester/covoar/ObjdumpProcessor.cc
> @@ -236,7 +236,8 @@ namespace Coverage {
>    void ObjdumpProcessor::getFile(
>      std::string fileName,
>      rld::process::tempfile& objdumpFile,
> -    rld::process::tempfile& err
> +    rld::process::tempfile& err,
> +    bool debug
>      )
>    {
>      rld::process::status        status;
> @@ -245,11 +246,13 @@ namespace Coverage {
>                                           fileName };
>      try
>      {
> -      status = rld::process::execute( TargetInfo->getObjdump(),
> -                                      args, objdumpFile.name(), err.name() );
> -      if ( (status.type != rld::process::status::normal)
> -           || (status.code != 0) ) {
> -        throw rld::error( "Objdump error", "generating objdump" );
> +      if (!debug || FileIsNewer( fileName.c_str(), objdumpFile.name().c_str() )) {
what's this !debug

Relying on timestamps can be tricky. You all should consider how to
create a proper cache of files, probably in the local directory from
which this tool is run, that updates based on hashed contents of the
source files rather than timestamps.  This is for your TODO list.

> +        status = rld::process::execute( TargetInfo->getObjdump(),
> +                                        args, objdumpFile.name(), err.name() );
> +        if ( (status.type != rld::process::status::normal)
> +             || (status.code != 0) ) {
> +          throw rld::error( "Objdump error", "generating objdump" );
> +        }
>        }
>      } catch( rld::error& err )
>        {
> @@ -325,7 +328,8 @@ namespace Coverage {
>    void ObjdumpProcessor::load(
>      ExecutableInfo* const    executableInformation,
>      rld::process::tempfile&  objdumpFile,
> -    rld::process::tempfile&  err
> +    rld::process::tempfile&  err,
> +    bool                     debug
>    )
>    {
>      std::string     currentSymbol = "";
> @@ -348,9 +352,9 @@ namespace Coverage {
>
>      // Obtain the objdump file.
>      if ( !executableInformation->hasDynamicLibrary() )
> -      getFile( executableInformation->getFileName(), objdumpFile, err );
> +      getFile( executableInformation->getFileName(), objdumpFile, err, debug );
>      else
> -      getFile( executableInformation->getLibraryName(), objdumpFile, err );
> +      getFile( executableInformation->getLibraryName(), objdumpFile, err, debug );
>
>      while ( true ) {
>        // Get the line.
> diff --git a/tester/covoar/ObjdumpProcessor.h b/tester/covoar/ObjdumpProcessor.h
> index c75755d..4dab001 100644
> --- a/tester/covoar/ObjdumpProcessor.h
> +++ b/tester/covoar/ObjdumpProcessor.h
> @@ -103,7 +103,8 @@ namespace Coverage {
>       */
>      void getFile( std::string fileName,
>                    rld::process::tempfile& dmp,
> -                  rld::process::tempfile& err );
> +                  rld::process::tempfile& err,
> +                  bool debug = false );
>
>      /*!
>       *  This method fills the objdumpList list with all the
> @@ -122,7 +123,8 @@ namespace Coverage {
>      void load(
>        ExecutableInfo* const executableInformation,
>        rld::process::tempfile& dmp,
> -      rld::process::tempfile& err
> +      rld::process::tempfile& err,
> +      bool debug
>      );
>
>      /*!
> diff --git a/tester/covoar/app_common.cc b/tester/covoar/app_common.cc
> index 8b490ed..0f3c8e2 100644
> --- a/tester/covoar/app_common.cc
> +++ b/tester/covoar/app_common.cc
> @@ -116,3 +116,13 @@ bool ReadUntilFound( FILE *file, const char *line )
>    } while (1);
>  }
>
> +std::string GetFileNameFromPath( const std::string& path )
> +{
> +  size_t idx = path.rfind('/', path.length());
Are there any library helpers that can do this?

What happens if you run this on windows? (I know the answer, this
stuff doesn't work on windows. But it doesn't mean we shouldn't
consider cross-platform issues?)

> +  if (idx == std::string::npos) {
> +    return "";
> +  }
> +
> +  return path.substr(idx + 1, path.length() - idx);
> +}
> +
> diff --git a/tester/covoar/app_common.h b/tester/covoar/app_common.h
> index ac32bbd..21e8cfa 100644
> --- a/tester/covoar/app_common.h
> +++ b/tester/covoar/app_common.h
> @@ -30,5 +30,6 @@ extern char                         inputBuffer2[MAX_LINE_LENGTH];
>  bool FileIsNewer( const char *f1, const char *f2 );
>  bool FileIsReadable( const char *f1 );
>  bool ReadUntilFound( FILE *file, const char *line );
> +std::string GetFileNameFromPath( const std::string& path );
>
>  #endif
> diff --git a/tester/covoar/covoar.cc b/tester/covoar/covoar.cc
> index bf95cb4..6b2082d 100644
> --- a/tester/covoar/covoar.cc
> +++ b/tester/covoar/covoar.cc
> @@ -373,8 +373,25 @@ int covoar(
>        exe->setLoadAddress( objdumpProcessor->determineLoadAddress( exe ) );
>      }
>
> -    // Load the objdump for the symbols in this executable.
> -    objdumpProcessor->load( exe, objdumpFile, err );
> +    if (debug) {
> +      std::string name;
> +
> +      if ( !exe->hasDynamicLibrary() ) {
> +        name = exe->getFileName();
> +      } else {
> +        name = exe->getLibraryName();
> +      }
> +
> +      name = GetFileNameFromPath( name );
> +      name = buildTarget + "-" + name;
> +      name.insert( 0, "/tmp/" );
> +
> +      rld::process::tempfile namedObjdumpFile( name, ".dmp", true );
> +      objdumpProcessor->load( exe, namedObjdumpFile, err, debug );
> +    } else {
> +      // Load the objdump for the symbols in this executable.
> +      objdumpProcessor->load( exe, objdumpFile, err, debug );
> +    }
>    }
>
>    //
> @@ -486,8 +503,6 @@ int covoar(
>
>    //Leave tempfiles around if debug flag (-d) is enabled.
>    if ( debug ) {
> -    objdumpFile.override( "objdump_file" );
> -    objdumpFile.keep();
>      err.override( "objdump_exec_log" );
>      err.keep();
>      syms.override( "symbols_list" );
> --
> 2.27.0
>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel


More information about the devel mailing list