On Aug 27, 2014, at 9:35 PM, Scott Duplichan <[email protected]> wrote:

> A couple of more points on this unusual policy of avoiding static functions:
>  
> 1) Wouldn't it make more sense for the debugger vendor to fix their bug 
> rather than endlessly code around it?
> 2) If static functions are not banned, then why are they getting removed?
>  
> I view it as an unsound engineering practice and unprofessional behavior for 
> employees of Intel to drive static functions out of an important open source 
> project because of a problem with some unnamed tool vendor. What other open 
> source project solves debugger problems this way? Where else in the world is 
> use of static functions banned discouraged in modern C code? The more UEFI 
> code differs from mainstream C code, the more difficult it is to get new 
> engineers interested in developing it.
>  

Since there is no dynamic linking in EFI the use of static functions in a 
driver or application has limited use. If you want to hide your code from 
yourself then static makes sense. The use of static probably makes more sense 
for private functions and globals in libraries, or in just porting existing 
code.

> Actually I think this debugger problem is an urban legend.

I seem to remember, it was 10+ years ago, that it was VS2003. It was probably a 
bug in the toolchain related to non standard usage for embedded development. 
Thus this rule ended up in the original Tiano coding style document, and it has 
probably been copied into subsequent versions (I only wrote the 1st one). 

Also since there is no dynamic linking it is possible that each module has a 
different instance of the same library function at a different address. I’ve 
seen this confuse debuggers in the past, usually due to a single symbol file 
having N instances of code at different addressees. Sometimes the issue is 
related to the debugger UI. 

Your debugger may vary (EmbeddedPkg breaking in at the BDS UI):
(lldb) br set -n DebugAssert
Breakpoint 2: 74 locations.
(lldb) br list
Current breakpoints:
1: name = 'SecGdbScriptBreak', locations = 1, resolved = 1, hit count = 72
    Breakpoint commands:
      return lldbefi.LoadEmulatorEfiSymbols(frame, bp_loc, internal_dict)

  1.1: where = Host`SecGdbScriptBreak + 18 at Host.c:1143, address = 
0x000000010000e402, resolved, hit count = 72 

2: name = 'DebugAssert', locations = 74, resolved = 72, hit count = 0
  2.1: where = Host`DebugAssert + 15 at DebugLib.c:73, address = 
0x000000010000729f, resolved, hit count = 0 
  2.2: where = CarbonCore`DebugAssert + 57 at Debugging.c:100, address = 
0x00007fff82585033, resolved, hit count = 0 
  2.3: where = EmuSec.dll`DebugAssert + 15 at DebugLib.c:136, address = 
0x0000000102022641, resolved, hit count = 0 
  2.4: where = PeiCore.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x0000000102024a03, resolved, hit count = 0 
  2.5: where = ReportStatusCodeRouterPei.dll`DebugAssert + 27 at 
DebugLib.c:271, address = 0x0000000102039373, resolved, hit count = 0 
  2.6: where = StatusCodeHandlerPei.dll`DebugAssert + 27 at DebugLib.c:271, 
address = 0x000000010203bb53, resolved, hit count = 0 
  2.7: where = PcdPeim.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x00000001020331cf, resolved, hit count = 0 
  2.8: where = BootModePei.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010204023f, resolved, hit count = 0 
  2.9: where = AutoScanPei.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x0000000102042237, resolved, hit count = 0 
  2.10: where = PeiCore.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x0000000000000aa3, unresolved, hit count = 0 
  2.11: where = FirmwareVolumePei.dll`DebugAssert + 27 at DebugLib.c:271, 
address = 0x0000000106fee1db, resolved, hit count = 0 
  2.12: where = FlashMapPei.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x0000000106feb00f, resolved, hit count = 0 
  2.13: where = ThunkPpiToProtocolPei.dll`DebugAssert + 27 at DebugLib.c:271, 
address = 0x0000000106fe8137, resolved, hit count = 0 
  2.14: where = FaultTolerantWritePei.dll`DebugAssert + 27 at DebugLib.c:271, 
address = 0x0000000106fe4d47, resolved, hit count = 0 
  2.15: where = PeiVariable.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x0000000106fe1163, resolved, hit count = 0 
  2.16: where = DxeIpl.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x0000000106fd97bf, resolved, hit count = 0 
  2.17: where = DxeCore.dll`DebugAssert + 15 at DebugLib.c:136, address = 
0x0000000106f8ccca, resolved, hit count = 0 
  2.18: where = DxeCore.dll`DebugAssert + 15 at DebugLib.c:136, address = 
0x0000000000002a8a, unresolved, hit count = 0 
  2.19: where = PcdDxe.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010665c4f3, resolved, hit count = 0 
  2.20: where = Metronome.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010666429f, resolved, hit count = 0 
  2.21: where = ReportStatusCodeRouterRuntimeDxe.dll`DebugAssert + 27 at 
DebugLib.c:271, address = 0x000000010664844b, resolved, hit count = 0 
  2.22: where = RealTimeClock.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x0000000106642233, resolved, hit count = 0 
  2.23: where = EmuReset.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x0000000106639233, resolved, hit count = 0 
  2.24: where = RuntimeDxe.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x0000000106628f5f, resolved, hit count = 0 
  2.25: where = FwBlockService.dll`DebugAssert + 27 at DebugLib.c:271, address 
= 0x00000001066151df, resolved, hit count = 0 
  2.26: where = SecurityStubDxe.dll`DebugAssert + 27 at DebugLib.c:271, address 
= 0x000000010660d67f, resolved, hit count = 0 
  2.27: where = EbcDxe.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x00000001065ed2bb, resolved, hit count = 0 
  2.28: where = NullMemoryTestDxe.dll`DebugAssert + 27 at DebugLib.c:271, 
address = 0x000000010663c233, resolved, hit count = 0 
  2.29: where = EmuThunk.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x00000001061e4233, resolved, hit count = 0 
  2.30: where = VariableRuntimeDxe.dll`DebugAssert + 27 at DebugLib.c:271, 
address = 0x00000001061b2aeb, resolved, hit count = 0 
  2.31: where = SerialDxe.dll`DebugAssert + 4 at DebugLib.c:73, address = 
0x00000001061c2492, resolved, hit count = 0 
  2.32: where = DevicePathDxe.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x0000000106148603, resolved, hit count = 0 
  2.33: where = SmbiosDxe.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010615ba17, resolved, hit count = 0 
  2.34: where = HiiDatabase.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010af860c3, resolved, hit count = 0 
  2.35: where = PrintDxe.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010afb1393, resolved, hit count = 0 
  2.36: where = DpcDxe.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010afa7b7f, resolved, hit count = 0 
  2.37: where = StatusCodeHandlerRuntimeDxe.dll`DebugAssert + 15 at 
DebugLib.c:136, address = 0x000000010af740d2, resolved, hit count = 0 
  2.38: where = Cpu.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010af6132f, resolved, hit count = 0 
  2.39: where = FaultTolerantWriteDxe.dll`DebugAssert + 27 at DebugLib.c:271, 
address = 0x000000010af451a7, resolved, hit count = 0 
  2.40: where = PlatformSmbiosDxe.dll`DebugAssert + 27 at DebugLib.c:271, 
address = 0x000000010af3cad3, resolved, hit count = 0 
  2.41: where = SetupBrowser.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010aebb3db, resolved, hit count = 0 
  2.42: where = EmuTimer.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010af4c29f, resolved, hit count = 0 
  2.43: where = MonotonicCounterRuntimeDxe.dll`DebugAssert + 27 at 
DebugLib.c:271, address = 0x000000010aef1287, resolved, hit count = 0 
  2.44: where = CapsuleRuntimeDxe.dll`DebugAssert + 27 at DebugLib.c:271, 
address = 0x000000010aedd87f, resolved, hit count = 0 
  2.45: where = DisplayEngine.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010ae5a4d3, resolved, hit count = 0 
  2.46: where = BdsDxe.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010ad69063, resolved, hit count = 0 
  2.47: where = WatchdogTimer.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010aeb3f03, resolved, hit count = 0 
  2.48: where = ConPlatformDxe.dll`DebugAssert + 27 at DebugLib.c:271, address 
= 0x000000010ae7e213, resolved, hit count = 0 
  2.49: where = ConSplitterDxe.dll`DebugAssert + 27 at DebugLib.c:271, address 
= 0x000000010ae4db5b, resolved, hit count = 0 
  2.50: where = GraphicsConsoleDxe.dll`DebugAssert + 27 at DebugLib.c:271, 
address = 0x000000010ae6e253, resolved, hit count = 0 
  2.51: where = TerminalDxe.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010adca41f, resolved, hit count = 0 
  2.52: where = DiskIoDxe.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010ae47a17, resolved, hit count = 0 
  2.53: where = PartitionDxe.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010adc1a17, resolved, hit count = 0 
  2.54: where = EnglishDxe.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010ae451d7, resolved, hit count = 0 
  2.55: where = PciBusDxe.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010ada9ba3, resolved, hit count = 0 
  2.56: where = ScsiBus.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010ae767a7, resolved, hit count = 0 
  2.57: where = ScsiDisk.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010ad5f5fb, resolved, hit count = 0 
  2.58: where = IdeBusDxe.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010ad44743, resolved, hit count = 0 
  2.59: where = EmuBusDriver.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010ad9ea17, resolved, hit count = 0 
  2.60: where = EmuGopDxe.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010ad52a17, resolved, hit count = 0 
  2.61: where = EmuSimpleFileSystem.dll`DebugAssert + 27 at DebugLib.c:271, 
address = 0x000000010ad59a17, resolved, hit count = 0 
  2.62: where = EmuBlockIo.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010ada3a17, resolved, hit count = 0 
  2.63: where = EmuSnpDxe.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010ad364d7, resolved, hit count = 0 
  2.64: where = ArpDxe.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010ad3ca1b, resolved, hit count = 0 
  2.65: where = Dhcp4Dxe.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010ad14c23, resolved, hit count = 0 
  2.66: where = Ip4ConfigDxe.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010ad09b7b, resolved, hit count = 0 
  2.67: where = Ip4Dxe.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010ad2446b, resolved, hit count = 0 
  2.68: where = MnpDxe.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010ace15d3, resolved, hit count = 0 
  2.69: where = VlanConfigDxe.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010acd65d3, resolved, hit count = 0 
  2.70: where = Mtftp4Dxe.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010acc9e2f, resolved, hit count = 0 
  2.71: where = Tcp4Dxe.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010ac9ab93, resolved, hit count = 0 
  2.72: where = Udp4Dxe.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010acbb927, resolved, hit count = 0 
  2.73: where = Fat.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010ac89a17, resolved, hit count = 0 
  2.74: where = DriverSample.dll`DebugAssert + 27 at DebugLib.c:271, address = 
0x000000010ac2cc67, resolved, hit count = 0 


Thanks,

Andrew Fish

> Debuggers generally process Microsoft PDB files using code supplied by 
> Microsoft. If either the PDB contents or the Microsoft supplied processing 
> code had a problem, it would likely show up everywhere Microsoft build tools 
> are used.
>  
> Thanks,
> Scott
> From: Qiu, Shumin [mailto:[email protected]] 
> Sent: Wednesday, August 27, 2014 09:40 PM
> To: Carsey, Jaben
> Cc: [email protected]
> Subject: [edk2] [Patch]ShellPkg: Remove 'STATIC' from function declarations 
> to avoid source level debugging problem
>  
> Hi Jaben,
> Can you help to review this patch? Internal linkage (ie. STATIC) functions 
> have caused problems with source level debugging before, so we generally 
> avoid STATIC in ShellPkg.
>  
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Qiu Shumin <[email protected]>
>  
> Thanks
> Shumin
>  
> ------------------------------------------------------------------------------
> Slashdot TV.  
> Video for Nerds.  Stuff that matters.
> http://tv.slashdot.org/_______________________________________________
> edk2-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/edk2-devel

------------------------------------------------------------------------------
Slashdot TV.  
Video for Nerds.  Stuff that matters.
http://tv.slashdot.org/
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to