Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(318)

Issue 21037004: Fix HasResult method of LTemplateInstruction to properly handle LCheckSmi (Closed)

Created:
7 years, 4 months ago by danno
Modified:
7 years, 4 months ago
Reviewers:
Sven Panne
CC:
v8-dev
Visibility:
Public.

Description

Fix HasResult method of LTemplateInstruction to properly handle LCheckSmi LCheckSmi sometimes has a result register and sometimes not, even though its LTemplateInstruction alwasys has room for one. Debug output use HasResult to determine whether it was ok to de-ref result(), but HasResult doesn't check for the case where LTemplateInstruction has a result but it's NULL. R=svenpanne@chromium.org Committed: http://code.google.com/p/v8/source/detail?r=15931

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+12 lines, -12 lines) Patch
M src/arm/lithium-arm.h View 2 chunks +3 lines, -3 lines 0 comments Download
M src/ia32/lithium-ia32.h View 2 chunks +3 lines, -3 lines 0 comments Download
M src/mips/lithium-mips.h View 2 chunks +3 lines, -3 lines 0 comments Download
M src/x64/lithium-x64.h View 2 chunks +3 lines, -3 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
danno
PTAL
7 years, 4 months ago (2013-07-29 10:58:10 UTC) #1
Sven Panne
LGTM. One can perhaps guard set_result() and result() with ASSERTs that the NULL case doesn't ...
7 years, 4 months ago (2013-07-29 11:53:00 UTC) #2
danno
Unfortunately, allowing NULL as a value result of CheckSmi seems to be baked in, it ...
7 years, 4 months ago (2013-07-29 11:56:30 UTC) #3
danno
7 years, 4 months ago (2013-07-29 11:57:52 UTC) #4
Message was sent while issue was closed.
Committed patchset #1 manually as r15931 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698