Date: Tue, 16 Oct 2001 11:34:20 -0700 From: "Grover, Andrew" <andrew.grover@intel.com> To: "'Peter Wemm'" <peter@wemm.org>, Mike Smith <msmith@FreeBSD.ORG> Cc: Marcel Moolenaar <marcel@xcllnt.net>, ia64@FreeBSD.ORG Subject: RE: Faulty ACPI debug code [was: Re: Booting a dual ...] Message-ID: <59885C5E3098D511AD690002A5072D3C42D66E@orsmsx111.jf.intel.com>
next in thread | raw e-mail | index | archive | help
Hi Peter,
Thanks for the problem reports. Our code can't use GCC extensions and
maintain ANSI C conformance, but it sure looks like using them briefly and
cleaning up the code would be a good thing to do...;-)
Regards -- Andy
> -----Original Message-----
> From: Peter Wemm [mailto:peter@wemm.org]
> Sent: Sunday, October 14, 2001 11:37 PM
> To: Mike Smith
> Cc: Marcel Moolenaar; ia64@FreeBSD.ORG; Andrew.Grover@intel.com
> Subject: Re: Faulty ACPI debug code [was: Re: Booting a dual ...]
>
>
> Mike Smith wrote:
> >
> > I've copied the Intel ACPI CA folks on this, since I'm sure
> they'll be
> > interested.
>
> While on the subject, If you add printf format checking,
> there are some
> *nasty* problems in the debug printfs. They fixed about 2/3
> of them a few
> months ago but left the others alone. They're still trying to printf
> a 64 bit int with %p on the i386 platform (which causes stack
> misalignment).
>
> ie:
> --- acutils.h 2001/10/04 23:12:11 1.1.1.11
> +++ acutils.h 2001/10/15 06:00:36
> @@ -436,5 +436,5 @@
> ACPI_DEBUG_PRINT_INFO *DbgInfo,
> char *Format,
> - ...);
> + ...) __attribute__ ((format (printf, 4, 5)));
>
> void
> @@ -444,5 +444,5 @@
> ACPI_DEBUG_PRINT_INFO *DbgInfo,
> char *Format,
> - ...);
> + ...) __attribute__ ((format (printf, 4, 5)));
>
>
> Additional warnings about the function in question:
>
> dsopcode.c: In function `AcpiDsGetRegionArguments':
> dsopcode.c:325: warning: char format, different type arg (arg 5)
>
> Line 325:
> ACPI_DEBUG_PRINT ((ACPI_DB_EXEC, "[%4.4s] OpRegion Init
> at AML %p[%x]\n",
> &Node->Name, ExtraDesc->Extra.AmlStart,
> *(UINT32*) ExtraDesc->Extra.AmlStart));
>
> typedef struct acpi_node
> {
> UINT32 Name; /* ACPI Name,
> always 4 chars per ACPI spec */
> } ACPI_NAMESPACE_NODE;
>
> This is passing an &UINT32 instead of 'char *'. This had better be
> little endian.
>
> A sample of more problems:
>
> evevent.c:559: warning: format argument is not a pointer (arg 6)
> evevent.c:559: warning: format argument is not a pointer (arg 8)
> ACPI_DEBUG_PRINT ((ACPI_DB_INFO, "GPE registers: %X@%p
> (Blk0) %X@%p (Blk1)\n",
> Gpe0RegisterCount, AcpiGbl_FADT->XGpe0Blk.Address,
> Gpe1RegisterCount,
> AcpiGbl_FADT->XGpe1Blk.Address));
> XGpe0Blk.Address and XGpe1Blk.Address are 64 bit integers,
> not pointers.
> This causes stack misalignment.
>
> exdump.c:348: warning: long int format, different type arg (arg 5)
> exdump.c:366: warning: long int format, different type arg (arg 5)
> ACPI_DEBUG_PRINT_RAW ((ACPI_DB_INFO, " value is [%ld]",
>
> EntryDesc->Integer.Value));
> The above two are moderately harmless unless you are compiling on
> gcc where long == 64 bit.
>
> exdump.c:491: warning: format argument is not a pointer (arg 5)
> ACPI_DEBUG_PRINT_RAW ((ACPI_DB_INFO, " base %p
> Length %X\n",
> EntryDesc->Region.Address, EntryDesc->Region.Length));
> This is a fatal UINT64 / 32 bit %p mixup.
>
> exprep.c:434: warning: char format, different type arg (arg 6)
> exprep.c:542: warning: char format, different type arg (arg 6)
> exprep.c:633: warning: char format, different type arg (arg 6)
> ACPI_DEBUG_PRINT ((ACPI_DB_INFO, "set NamedObj %p (%4.4s)
> val = %p\n",
> Node, &(Node->Name), ObjDesc));
> More &UINT32 vs char *
>
> exresolv.c:239: warning: long unsigned int format, different
> type arg (arg 11)
> relatively harmless long / int mixup.
>
> exresop.c:248: warning: unsigned int format, pointer arg (arg 5)
> ACPI_DEBUG_PRINT ((ACPI_DB_ERROR, "Internal -
> null stack entry at %X\n",
> StackPtr));
> This should actually use %p
>
> hwregs.c:573: warning: format argument is not a pointer (arg 6)
> ACPI_DEBUG_PRINT ((ACPI_DB_IO, "PM2 control: Read %X
> from %p\n",
> RegisterValue, ACPI_GET_ADDRESS
> (AcpiGbl_FADT->XPm2CntBlk.Address)));
> Another fatal (for debugging) UINT64 / %p mixup
>
> hwregs.c:583: warning: format argument is not a pointer (arg 6)
> ACPI_DEBUG_PRINT ((ACPI_DB_IO, "About to write
> %04X to %p\n", RegisterValue,
> AcpiGbl_FADT->XPm2CntBlk.Address));
> same again
>
> hwregs.c:597: warning: format argument is not a pointer (arg 6)
> ACPI_DEBUG_PRINT ((ACPI_DB_IO, "PM_TIMER: Read %X from %p\n",
> RegisterValue, ACPI_GET_ADDRESS
> (AcpiGbl_FADT->XPmTmrBlk.Address)));
> and again...
>
> nsdump.c:570: warning: unsigned int format, different type arg (arg 6)
> ACPI_DEBUG_PRINT_RAW ((ACPI_DB_TABLES, " HID:
> %.8X, ADR: %.8X, Status: %x\n",
> Info.HardwareId, Info.Address,
> Info.CurrentStatus));
> This is an interesting one... Info.HardwareId is NATIVE_CHAR
> HardwareId[9];
> It should probably be %p.
>
> This is a sample of the problems that using printf type checking
> turns up. I have only included a small fraction of the instances of
> each problem as an example only. Some only mess up debugging output,
> and cause lots of wasted time due to wild goose chases when bogus
> values are reported. (printf %p of a uint64 always prints 0 on i386
> with gcc (eg: on freebsd and linux))
>
> Cheers,
> -Peter
> --
> Peter Wemm - peter@FreeBSD.org; peter@yahoo-inc.com;
> peter@netplex.com.au
> "All of this is for nothing if we don't go to the stars" - JMS/B5
>
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-ia64" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?59885C5E3098D511AD690002A5072D3C42D66E>