public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Marvin Häuser" <mhaeuser@posteo.de>
To: "Zeng, Star" <star.zeng@intel.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>,
	"Yao, Jiewen" <jiewen.yao@intel.com>
Subject: Re: [edk2-devel] Possibly incorrect size in memory profile structure
Date: Tue, 22 Jun 2021 11:55:48 +0000	[thread overview]
Message-ID: <ba3b47ea-c0f4-82fa-cdcb-e5bd7179d9d8@posteo.de> (raw)
In-Reply-To: <DM6PR11MB40589A7025BF1E14EFE7B61BE3099@DM6PR11MB4058.namprd11.prod.outlook.com>

Ahhh, I see it. The size does to describe the data structure that is 
existent at that point in time, but the one that is created by 
MemoryProfileCopyData() later, and the alignment is applied because it 
ends up as a flat structure array. Makes sense, thank you a lot!

Best regards,
Marvin

On 22.06.21 12:20, Zeng, Star wrote:
> Not sure I can understand your question correctly. But try best to recall and comment since not on it for long time. 😊
>
> [2], [3] and [4] are all for interior storage management by MEMORY_PROFILE_DRIVER_INFO_DATA, MEMORY_PROFILE_DRIVER_INFO_DATA.PdbString points to the PDB string.
>
> Exterior storage are managed by ProfileProtocolGetData() and MemoryProfileCopyData() from interior storage, 8B alignment are for exterior storage, PDB string is at the end of MEMORY_PROFILE_DRIVER_INFO structure.
>
>
> Thanks,
> Star
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Marvin Häuser
> Sent: 2021年6月19日 18:56
> To: Zeng, Star <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> Subject: [edk2-devel] Possibly incorrect size in memory profile structure
>
> Good day,
>
> I have a question regarding a part of the memory profiling code. Namely this piece of code [1] introduced storing the PDB file name among the driver information. This is implement by a string pointer in the "MEMORY_PROFILE_DRIVER_INFO_DATA" structure (which embeds the "MEMORY_PROFILE_DRIVER_INFO" structure [2]). The length of the embedded "MEMORY_PROFILE_DRIVER_INFO" instance is set to the header size plus the 8B-aligned size of the PDB name [3], albeit its storage is not used for the PDB file name, and the storage that is used instead is not aligned by 8B [4]. Ominously, the interior structure does have a comment that indicates it is (or was) supposed to hold the PDB file name at some point [5].
>
> I do not see that concept being used, and instead see the previously described way of storage, so I believe the aligned size is a forgotten piece from a refactoring. I imagine early on, the interior structure was supposed to hold the PDB name, and the alignment was needed to satisfy the following data's requirements. However, the change to the exterior storage should make this superfluous, and the specified size seems to be incorrect in all cases.
>
> Can you please give this a quick look and help me determine whether this is a bug? Thank you for your time!
>
> Best regards,
> Marvin
>
> [1]
> https://github.com/tianocore/edk2/commit/1d60fe96422206d37e1d74198bb11b2cf6195157#diff-b42ade68f10fa42dfa25570f0f9a165db4b974877c98d8845e384a40252ec220R407-R428
>
> [2]
> https://github.com/tianocore/edk2/blob/a63914d3f603580e5aeceb5edbafe56688210141/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c#L25
>
> [3]
> https://github.com/tianocore/edk2/blob/a63914d3f603580e5aeceb5edbafe56688210141/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c#L417
>
> [4]
> https://github.com/tianocore/edk2/blob/a63914d3f603580e5aeceb5edbafe56688210141/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c#L404
>
> [5]
> https://github.com/tianocore/edk2/blob/a63914d3f603580e5aeceb5edbafe56688210141/MdeModulePkg/Include/Guid/MemoryProfile.h#L59
>
>
> 
>
>


      reply	other threads:[~2021-06-22 11:55 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-19 10:56 Possibly incorrect size in memory profile structure Marvin Häuser
2021-06-22 10:20 ` [edk2-devel] " Zeng, Star
2021-06-22 11:55   ` Marvin Häuser [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ba3b47ea-c0f4-82fa-cdcb-e5bd7179d9d8@posteo.de \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox