Hi Drew,
On 12/19/18 11:10 AM, Andrew Jones wrote:
> On Tue, Dec 18, 2018 at 06:51:20PM +0100, Philippe Mathieu-Daudé wrote:
>> GCC 8 added a -Wstringop-truncation warning:
>>
>> The -Wstringop-truncation warning added in GCC 8.0 via r254630 for
>> bug 81117 is specifically intended to highlight likely unintended
>> uses of the strncpy function that truncate the terminating NUL
>> character from the source string.
>>
>> This new warning leads to compilation failures:
>>
>> CC hw/acpi/core.o
>> In function 'acpi_table_install', inlined from 'acpi_table_add' at
>> qemu/hw/acpi/core.c:296:5:
>> qemu/hw/acpi/core.c:184:9: error: 'strncpy' specified bound 4 equals
>> destination size [-Werror=stringop-truncation]
>> strncpy(ext_hdr->sig, hdrs->sig, sizeof ext_hdr->sig);
>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> make: *** [qemu/rules.mak:69: hw/acpi/core.o] Error 1
>>
>> Use the QEMU_NONSTRING attribute, since ACPI tables don't require the
>> strings to be NUL-terminated.
>
> Aren't we always starting with zero-initialized structures in ACPI code?
> If so, then we should be able to change the strncpy's to memcpy's.
The first call zero-initializes, but then we call realloc():
/* We won't fail from here on. Initialize / extend the globals. */
if (acpi_tables == NULL) {
acpi_tables_len = sizeof(uint16_t);
acpi_tables = g_malloc0(acpi_tables_len);
}
acpi_tables = g_realloc(acpi_tables, acpi_tables_len +
ACPI_TABLE_PFX_SIZE +
sizeof dfl_hdr + body_size);
ext_hdr = (struct acpi_table_header *)(acpi_tables +
acpi_tables_len);
So memcpy() isn't enough.
I can resend the previous patch which uses strpadcpy() if you prefer,
Igor already reviewed it:
https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg04406.html
>>
>> Suggested-by: Michael S. Tsirkin <m...@redhat.com>
>> Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com>
>> ---
>> hw/acpi/core.c | 8 ++++----
>> include/hw/acpi/acpi-defs.h | 8 ++++----
>> 2 files changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/acpi/core.c b/hw/acpi/core.c
>> index aafdc61648..f60f750c3d 100644
>> --- a/hw/acpi/core.c
>> +++ b/hw/acpi/core.c
>> @@ -35,14 +35,14 @@
>> struct acpi_table_header {
>> uint16_t _length; /* our length, not actual part of the hdr */
>> /* allows easier parsing for fw_cfg clients */
>> - char sig[4]; /* ACPI signature (4 ASCII characters) */
>> + char sig[4] QEMU_NONSTRING; /* ACPI signature (4 ASCII characters) */
>> uint32_t length; /* Length of table, in bytes, including
>> header */
>> uint8_t revision; /* ACPI Specification minor version # */
>> uint8_t checksum; /* To make sum of entire table == 0 */
>> - char oem_id[6]; /* OEM identification */
>> - char oem_table_id[8]; /* OEM table identification */
>> + char oem_id[6] QEMU_NONSTRING; /* OEM identification */
>> + char oem_table_id[8] QEMU_NONSTRING; /* OEM table identification */
>> uint32_t oem_revision; /* OEM revision number */
>> - char asl_compiler_id[4]; /* ASL compiler vendor ID */
>> + char asl_compiler_id[4] QEMU_NONSTRING; /* ASL compiler vendor ID */
>> uint32_t asl_compiler_revision; /* ASL compiler revision number */
>> } QEMU_PACKED;
>>
>> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
>> index af8e023968..3bf0bec8ba 100644
>> --- a/include/hw/acpi/acpi-defs.h
>> +++ b/include/hw/acpi/acpi-defs.h
>> @@ -43,7 +43,7 @@ enum {
>> struct AcpiRsdpDescriptor { /* Root System Descriptor Pointer */
>> uint64_t signature; /* ACPI signature, contains "RSD PTR "
>> */
>> uint8_t checksum; /* To make sum of struct == 0 */
>> - uint8_t oem_id [6]; /* OEM identification */
>> + uint8_t oem_id [6] QEMU_NONSTRING; /* OEM identification */
>> uint8_t revision; /* Must be 0 for 1.0, 2 for 2.0 */
>> uint32_t rsdt_physical_address; /* 32-bit physical address of RSDT */
>> uint32_t length; /* XSDT Length in bytes including hdr
>> */
>> @@ -62,10 +62,10 @@ typedef struct AcpiRsdpDescriptor AcpiRsdpDescriptor;
>> uint32_t length; /* Length of table, in bytes,
>> including header */ \
>> uint8_t revision; /* ACPI Specification minor version #
>> */ \
>> uint8_t checksum; /* To make sum of entire table == 0 */
>> \
>> - uint8_t oem_id [6]; /* OEM identification */ \
>> - uint8_t oem_table_id [8]; /* OEM table identification */ \
>> + uint8_t oem_id [6] QEMU_NONSTRING; /* OEM identification */ \
>> + uint8_t oem_table_id [8] QEMU_NONSTRING; /* OEM table identification
>> */ \
>> uint32_t oem_revision; /* OEM revision number */ \
>> - uint8_t asl_compiler_id [4]; /* ASL compiler vendor ID */ \
>> + uint8_t asl_compiler_id [4] QEMU_NONSTRING; /* ASL compiler vendor ID
>> */ \
>> uint32_t asl_compiler_revision; /* ASL compiler revision number */
>>
>>
>> --
>> 2.17.2
>>
>>
