From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from foss.arm.com (foss.arm.com [217.140.110.172])
 by mx.groups.io with SMTP id smtpd.web12.482.1589485548065050194
 for <devel@edk2.groups.io>;
 Thu, 14 May 2020 12:45:48 -0700
Authentication-Results: mx.groups.io;
 dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: ard.biesheuvel@arm.com)
Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14])
	by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id BE9691FB;
	Thu, 14 May 2020 12:45:46 -0700 (PDT)
Received: from [192.168.1.81] (unknown [10.37.8.255])
	by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 11C463F71E;
	Thu, 14 May 2020 12:45:42 -0700 (PDT)
Subject: Re: [edk2-devel] [PATCH V6 1/1] MdePkg: Fix SafeString performing assertions on runtime checks
To: devel@edk2.groups.io, cheptsov@ispras.ru,
 "Kinney, Michael D" <michael.d.kinney@intel.com>
Cc: Andrew Fish <afish@apple.com>, Bret Barkelew
 <bret.barkelew@microsoft.com>, "Brian J . Johnson" <brian.johnson@hpe.com>,
 "Chiu, Chasel" <chasel.chiu@intel.com>,
 "Justen, Jordan L" <jordan.l.justen@intel.com>,
 Laszlo Ersek <lersek@redhat.com>, Leif Lindholm <leif@nuviainc.com>,
 "Gao, Liming" <liming.gao@intel.com>, =?UTF-8?Q?Marvin_H=c3=a4user?=
 <mhaeuser@outlook.de>, "Zimmer, Vincent" <vincent.zimmer@intel.com>,
 "Gao, Zhichao" <zhichao.gao@intel.com>
References: <20200514092537.29609-1-cheptsov@ispras.ru>
 <20200514092537.29609-2-cheptsov@ispras.ru>
 <MN2PR11MB4461CC95E8C6660BFFA3106FD2BC0@MN2PR11MB4461.namprd11.prod.outlook.com>
 <B28C8285-E178-417F-A2ED-2465E3ACB1BD@ispras.ru>
 <MN2PR11MB4461DF6CF7F71085C716E87CD2BC0@MN2PR11MB4461.namprd11.prod.outlook.com>
 <EEAC21EB-5AB3-49AF-A894-4D9A026F2236@ispras.ru>
From: "Ard Biesheuvel" <ard.biesheuvel@arm.com>
Message-ID: <f28dc5a7-86b2-d022-924b-c0633eb40a3b@arm.com>
Date: Thu, 14 May 2020 21:45:41 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.7.0
MIME-Version: 1.0
In-Reply-To: <EEAC21EB-5AB3-49AF-A894-4D9A026F2236@ispras.ru>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit

On 5/14/20 8:59 PM, Vitaly Cheptsov via groups.io wrote:
> Mike,
> 
> The code you posted may inflict undefined behaviour is not valid C for 
> several reasons. The compiler is free to do whatever it desires. Please 
> refer to ISO/IEC 9899 for more details.
> 
> If applications cast raw pointers to typed pointers without checking 
> their alignment, well, god bless them :)
> My opinion is both the compiler and the hardware are welcome to do the 
> worst once your third line is discovered. On a number of CPUs such 
> addresses cannot be even represented in the first place.
> 
> Yet, once again it is out of the scope of the current problem.
> 

This has come up a couple of times in the past, and it is indeed a 
serious issue with the EDK2 codebase.

For example, in BaseLib we have

UINT16
EFIAPI
WriteUnaligned16 (
   OUT UINT16                    *Buffer,
   IN  UINT16                    Value
   );

which does not make any sense, since a UINT16* is defined as being 
aligned to 16 bits, and so the compiler is permitted to generate code 
assuming that constraint is never violated.

So the correct way would be to define this prototype as

UINT16
EFIAPI
WriteUnaligned16 (
   OUT VOID                      *Buffer,
   IN  UINT16                    Value
   );

and let the underlying implementation deal with the misalignment.

As I said, I'm sure this has come up in the past, and there may even be 
a bugzilla entry for it.