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

["Jordan Justen" <jordan.l.justen@intel.com>]

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
> > >