RE: [PATCH] acpi: acpica: dsutils: fixanoff-by-one index

[Zheng, Lv]

Hi,

> From: Seraphime Kirkovski
> 
> On Wed, Jun 07, 2017 at 03:14:46PM +0000, Moore, Robert wrote:
> > I believe that the rationale for this is that at that point in the code, it is *guaranteed* that
> there is at least one operand; therefore the -1 would always be valid.
> >
> > In the end, we just deleted that call to
> > acpi_db_display_argument_object.

Yes, the AcpiDbDisplayArgumentObject() invocation itself here is not proper.
It can mess up debugging messages.

In debugging console, AcpiDbDisplayArgumentObject() can be enabled to display stacked objects.
In single step mode, it dumps stacked objects on the debugging console for each step.
It means AcpiDbDisplayArgumentObject() should be invoked for each AcpiDsObjStackPush().
There are only 2 AcpiDsObjStackPush() invocations.
Both of them have have already been paired with AcpiDbDisplayArgumentObject() in AcpiDsCreateOperand().

This isolated AcpiDbDisplayArgumentObject() tries to find back missing stacked object debugging information.
			ACPI_DEBUG_PRINT((ACPI_DB_DISPATCH,
 					  "Argument previously created, already stacked\n"));
However there won't be such a missing object as all operands are created via AcpiDsCreateOperand().
Finally this invocation only generates redundant debugging logs on console:
 ArgObj: 00ADA610 Integer 0000000000000000
 ArgObj: 00ADA4F0 Integer 00000000FFFF00FF
 ArgObj: 00ADA4F0 Integer 00000000FFFF00FF <= redundant log generated by this line
 ArgObj: 00ADA580 Integer 0000000000000000
 ResultObj: 00ADA710 Integer 0000000000000000
So we just removed it.

> > I don't know if this change has made it into Linux yet.
> >
> The latest rc actually produces the UBSAN splat in my previous message.
> So I suppose, I have some buggy hardware/firmware.

It's in recent ACPICA release and hasn't been merged by Linux upstream.
It's still in community review list:
https://patchwork.kernel.org/project/linux-acpi/list/
named as:
[33/53] ACPICA: Dispatcher: Remove unnecessary call to debugger
You'll see it merged in the next 1-2 RCs if everything works smoothly.

Thanks
Lv