fix mSecureHashAlgorithms name clash, part two

[kraxel]

• NetworkPkg/DxeNetLib: drop GLOBAL_REMOVE_IF_UNREFERENCED
• MdePkg/DxeRngLib: drop GLOBAL_REMOVE_IF_UNREFERENCED

[ardbiesheuvel]

GLOBAL_REMOVE_IF_UNREFERENCED should only be used if the global objects in question are interchangeable, and can be merged into a single one at link time. This is appropriate for generated objects such as gEfiCallerIdGuid or gEfiCallerBaseName but this is something we rarely need in ordinary C code.

Case in point: these two occurrences, where the arrays are of different types, and the use of __declspec(selectany) on Windows will result in the objects to be merged into a single one, resulting in broken code.

Could we perhaps get rid of GLOBAL_REMOVE_IF_UNREFERENCED entirely?

@os-d @makubacki

[ardbiesheuvel]

Nit: this should be

static EFI_GUID * CONST mSecureHashAlgorithms[]

if we're changing the code anyway.

[ardbiesheuvel]

Actually, it appears that __declspec(selectany) is only used for VS2013 and older, which we no longer support.

So that a) makes this patch unnecessary, given that GLOBAL_REMOVE_IF_UNREFERENCED is never #define'd, and b) answers my previous question about removing it entirely, which is obviously the case if it is never set anymore.

[os-d]

Agreed with @ardbiesheuvel with getting rid of GLOBAL_REMOVE_IF_UNREFERENCED because it is unused. Looks like at the time the decision was to use the newly added /Gw flag: https://edk2.groups.io/g/devel/message/11616.

[mdkinney]

I agree. Base.h has the following:

//
// The Microsoft* C compiler can removed references to unreferenced data items
//  if the /OPT:REF linker option is used. We defined a macro as this is a
//  a non standard extension
//
#if defined (_MSC_VER) && _MSC_VER < 1800 && !defined (MDE_CPU_EBC)
///
/// Remove global variable from the linked image if there are no references to
/// it after all compiler and linker optimizations have been performed.
///
///
#define GLOBAL_REMOVE_IF_UNREFERENCED  __declspec(selectany)
#else
///
/// Remove the global variable from the linked image if there are no references
///  to it after all compiler and linker optimizations have been performed.
///
///
#define GLOBAL_REMOVE_IF_UNREFERENCED
#endif

The meaning of the check for _MSC_VER < 1800 can be found here at
https://learn.microsoft.com/en-us/cpp/overview/compiler-versions?view=msvc-170#version-macros

Which means that _declspec(selectany) was only used for VS2012 and older.

This commit removed support for VS2013 and older tool chains: 0363584

This means that GLOBAL_REMOVE_IF_UNREFERENCED is always defined to nothing and has no meaning.

This PR is not a critical bug fix because GLOBAL_REMOVE_IF_UNREFERENCED is always defined to nothing now.

A more complete fix would be to remove GLOBAL_REMOVE_IF_UNREFERENCED from all edk2 sources and simplify Base.h to unconditionally define GLOBAL_REMOVE_IF_UNREFERENCED to nothing to support downstream repos that may be using this macro and would need time to remove.

[ardbiesheuvel]

This PR is not a critical bug fix because GLOBAL_REMOVE_IF_UNREFERENCED is always defined to nothing now.

Agreed. I failed to notice this when I brought it up. So we should fix this, but this PR can be omitted from the stable tag.

A more complete fix would be to remove GLOBAL_REMOVE_IF_UNREFERENCED from all edk2 sources and simplify Base.h to unconditionally define GLOBAL_REMOVE_IF_UNREFERENCED to nothing to support downstream repos that may be using this macro and would need time to remove.

I can look into that. As part of that effort, any global variable that is only referenced from the source file that defines it should be made static as well.