Re: [Patch V4 06/10] EmulatorPkg: Fix XCODE5 lldb issues

["Michael D Kinney" <michael.d.kinney@intel.com>]

Jordan,

Thanks.  Yes.  Updating the Author sounds like the
right fix.  I will send an updated series soon.

Thanks,

Mike

> -----Original Message-----
> From: Justen, Jordan L
> Sent: Friday, August 16, 2019 11:21 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Andrew Fish
> <afish@apple.com>
> Subject: RE: [Patch V4 06/10] EmulatorPkg: Fix XCODE5
> lldb issues
> 
> On 2019-08-16 08:09:55, Kinney, Michael D wrote:
> > Jordan,
> >
> > It is not a typo.
> >
> > Andrew generated the XCODE specific changes, so they
> have been tested
> > by him.
> 
> In that case, is the git author for the patches set to
> Andrew? If it was, I would expect to see a line inside
> the email body with:
> 
> From: Andrew Fish <afish@apple.com>
> 
> Git does that for patches where the sender doesn't
> match the patch author.
> 
> You might want to rebase and run:
> 
> git commit --amend --author="Andrew Fish
> <afish@apple.com>" --reset-author
> 
> to change the author.
> 
> I expect you might want to add Reviewed-by for yourself
> on these patches to help speed things along. If Andrew
> authored the patch, you reviewed it, and with a quick
> review it looked good to me, I would probably add an
> Acked-by for some of the patches.
> 
> -Jordan
> 
> > I have also reviewed and tested the XCODE changes and
> verified that
> > all 6 combinations build and boot to shell (IA32/X64
> for
> > RELEASE/DEBUG/NOOPT). Since they are all related to
> making EmulatorPkg
> > work, we decided to fold them into the same patch set
> that was already
> > being reviewed.
> >
> > I also verified build and boot to shell for 6
> combinations on GCC5
> > (IA32/X64 for RELEASE/DEBUG/NOOPT) and the 12
> combinations of
> > VS2015/VS2017, IA323/X64, RELEASE/DEBUG/NOOPT.
> >
> > I have been working on some CI experiments using
> Azure Pipelines.
> > Here is a pointer to the build logs for all the
> combinations listed
> > above.
> >
> > https://dev.azure.com/mikekinney/edk2-
> ci/_build/results?buildId=312
> >
> > Mike
> >
> > > -----Original Message-----
> > > From: Justen, Jordan L
> > > Sent: Friday, August 16, 2019 12:41 AM
> > > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > > devel@edk2.groups.io
> > > Cc: Ni, Ray <ray.ni@intel.com>; Andrew Fish
> <afish@apple.com>
> > > Subject: Re: [Patch V4 06/10] EmulatorPkg: Fix
> XCODE5 lldb issues
> > >
> > > On 2019-08-15 19:14:33, Michael D Kinney wrote:
> > > > Fix scripts to support lldb symbolic debugging
> when
> > > using XCODE5 tool
> > > > chain.
> > > >
> > > > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > > > Cc: Ray Ni <ray.ni@intel.com>
> > > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > > Signed-off-by: Andrew Fish <afish@apple.com>
> > >
> > > Is this a Cc/Signed-off-by typo? (See also, patches
> 7- 10).
> > >
> > > This makes me wonder if you are taking advantage of
> the git commit
> > > -s switch to add your Signed-off-by.
> > >
> > > Also, I'm wondering if you are taking advantage of
> git- send-email
> > > automatically Cc'ing the addresses you listed in
> the commit message.
> > > (I thought it Cc'd for the author and Cc tags, but
> I wasn't sure
> > > about the Signed-off-by tag, and yet I see Andrew
> was Cc'd.)
> > >
> > > There's a couple long lines below. You could use \
> at the end of the
> > > line to split the .sh line. I think the cd can be a
> separate command
> > > in a shell script. (Not in
> > > make)
> > >
> > > I hope someone that uses the XCODE toolchain could
> review/check the
> > > XCODE patches.
> > >
> > > -Jordan
> > >
> > > > ---
> > > >  EmulatorPkg/Unix/lldbefi.py |  8 +++++---
> > > >  EmulatorPkg/build.sh        | 17 ++-------------
> --
> > > >  2 files changed, 7 insertions(+), 18 deletions(-
> )
> > > >
> > > > diff --git a/EmulatorPkg/Unix/lldbefi.py
> > > b/EmulatorPkg/Unix/lldbefi.py
> > > > index 218326b8cb..099192d8b5 100755
> > > > --- a/EmulatorPkg/Unix/lldbefi.py
> > > > +++ b/EmulatorPkg/Unix/lldbefi.py
> > > > @@ -346,6 +346,7 @@ def
> TypePrintFormating(debugger):
> > > >      debugger.HandleCommand("type summary add
> CHAR8 -
> > > -python-function lldbefi.CHAR8_TypeSummary")
> > > >      debugger.HandleCommand('type summary add --
> regex
> > > "CHAR8
> > > > \[[0-9]+\]" --python-function
> > > lldbefi.CHAR8_TypeSummary')
> > > >
> > > > +    debugger.HandleCommand('setting set frame-
> format
> > > "frame
> > > > + #${frame.index}: ${frame.pc}{
> > > > +
> > >
> ${module.file.basename}{:${function.name}()${function.p
> > > c-offset}}}{
> > > > + at ${line.file.fullpath}:${line.number}}\n"')
> > > >
> > > >  gEmulatorBreakWorkaroundNeeded = True
> > > >
> > > > @@ -381,15 +382,16 @@ def
> > > LoadEmulatorEfiSymbols(frame, bp_loc ,
> internal_dict):
> > > >      Error = lldb.SBError()
> > > >      FileNamePtr = frame.FindVariable
> > > ("FileName").GetValueAsUnsigned()
> > > >      FileNameLen = frame.FindVariable
> > > > ("FileNameLength").GetValueAsUnsigned()
> > > > +
> > > >      FileName =
> > > frame.thread.process.ReadCStringFromMemory
> > > (FileNamePtr, FileNameLen, Error)
> > > >      if not Error.Success():
> > > >          print "!ReadCStringFromMemory() did not
> find
> > > a %d byte C string at %x" % (FileNameLen,
> FileNamePtr)
> > > >          # make breakpoint command contiue
> > > > -
> frame.GetThread().GetProcess().Continue()
> > > > +        return False
> > > >
> > > >      debugger =
> frame.thread.process.target.debugger
> > > >      if frame.FindVariable
> > > ("AddSymbolFlag").GetValueAsUnsigned() == 1:
> > > > -        LoadAddress = frame.FindVariable
> > > ("LoadAddress").GetValueAsUnsigned()
> > > > +        LoadAddress = frame.FindVariable
> > > > + ("LoadAddress").GetValueAsUnsigned() - 0x240
> > > >
> > > >          debugger.HandleCommand ("target modules
> add
> > > %s" % FileName)
> > > >          print "target modules load --slid 0x%x
> %s" %
> > > (LoadAddress,
> > > > FileName) @@ -405,7 +407,7 @@ def
> > > LoadEmulatorEfiSymbols(frame, bp_loc ,
> internal_dict):
> > > >                      print
> "!lldb.target.RemoveModule
> > > (%s) FAILED" %
> > > > SBModule
> > > >
> > > >      # make breakpoint command contiue
> > > > -    frame.thread.process.Continue()
> > > > +    return False
> > > >
> > > >  def GuidToCStructStr (guid, Name=False):
> > > >    #
> > > > diff --git a/EmulatorPkg/build.sh
> > > b/EmulatorPkg/build.sh index
> > > > 60056e1b6c..35912a7775 100755
> > > > --- a/EmulatorPkg/build.sh
> > > > +++ b/EmulatorPkg/build.sh
> > > > @@ -209,21 +209,8 @@ fi
> > > >  if [[ "$RUN_EMULATOR" == "yes" ]]; then
> > > >    case `uname` in
> > > >      Darwin*)
> > > > -      #
> > > > -      # On Darwin we can't use dlopen, so we
> have to
> > > load the real PE/COFF images.
> > > > -      # This .gdbinit script sets a breakpoint
> that
> > > loads symbols for the PE/COFFEE
> > > > -      # images that get loaded in Host
> > > > -      #
> > > > -      if [[ "$CLANG_VER" == *-ccc-host-triple*
> ]]
> > > > -      then
> > > > -      # only older versions of Xcode support -
> ccc-
> > > host-tripe, for newer versions
> > > > -      # it is -target
> > > > -        cp
> $WORKSPACE/EmulatorPkg/Unix/lldbefi.py
> > >
> "$BUILD_OUTPUT_DIR/${BUILDTARGET}_$TARGET_TOOLS/$PROCES
> > > SOR"
> > > > -        cd $BUILD_ROOT_ARCH; /usr/bin/lldb --
> source
> > > $WORKSPACE/EmulatorPkg/Unix/lldbinit Host
> > > > -        exit $?
> > > > -      else
> > > > -        cp $WORKSPACE/EmulatorPkg/Unix/.gdbinit
> > >
> "$BUILD_OUTPUT_DIR/${BUILDTARGET}_$TARGET_TOOLS/$PROCES
> > > SOR"
> > > > -      fi
> > > > +      cd $BUILD_ROOT_ARCH; /usr/bin/lldb -o
> "command
> > > script import
> $WORKSPACE/EmulatorPkg/Unix/lldbefi.py" - o 'script
> > > lldb.debugger.SetAsync(True)' -o "run" ./Host
> > > > +      exit $?
> > > >        ;;
> > > >    esac
> > > >
> > > > --
> > > > 2.21.0.windows.1
> > > >