Page MenuHomePhabricator

Fully fix Bug #22115.
ClosedPublic

Authored by jhibbits on Jan 9 2015, 2:29 PM.

Details

Summary

In the previous commit, the register was saved, but space was not allocated.
This resulted in the parameter save area potentially clobbering r30, leading to
nasty results.

Diff Detail

Repository
rL LLVM

Event Timeline

jhibbits updated this revision to Diff 17951.Jan 9 2015, 2:29 PM
jhibbits retitled this revision from to Fully fix Bug #22115..
jhibbits updated this object.
jhibbits edited the test plan for this revision. (Show Details)
jhibbits added a reviewer: hfinkel.
jhibbits added a subscriber: Unknown Object (MLST).
hfinkel added inline comments.Jan 9 2015, 2:47 PM
lib/Target/PowerPC/PPCFrameLowering.cpp
1163 ↗(On Diff #17951)

This variable is only used inside the if statement. Why are you creating it out here?

lib/Target/PowerPC/PPCMachineFunctionInfo.h
123 ↗(On Diff #17951)

We don't initialize all of the member variables here, but I'd rather we did. Can you please initialize PICBasePointerSaveIndex here.

jhibbits added inline comments.Jan 9 2015, 3:26 PM
lib/Target/PowerPC/PPCFrameLowering.cpp
1163 ↗(On Diff #17951)

It's how the others do it (BPSI is identical, which is where I copied it from). I'll fix this one though.

1163 ↗(On Diff #17951)

D'oh, bad reading on my part. Ignore this. BPSI is used in the condition.

lib/Target/PowerPC/PPCMachineFunctionInfo.h
123 ↗(On Diff #17951)

Sure thing.

hfinkel edited edge metadata.Jan 9 2015, 3:27 PM

Thanks. With those changes, LGTM.

-Hal

  • Original Message -----

From: "Justin Hibbits" <jrh29@alumni.cwru.edu>
To: jrh29@alumni.cwru.edu, hfinkel@anl.gov
Cc: llvm-commits@cs.uiuc.edu
Sent: Friday, January 9, 2015 5:26:23 PM
Subject: Re: [PATCH] Fully fix Bug #22115.

================
Comment at: lib/Target/PowerPC/PPCFrameLowering.cpp:1163
@@ +1162,3 @@
+ // Only used in SVR4 32-bit.
+ int PBPSI = FI->getPICBasePointerSaveIndex();
+ if (FI->usesPICBase()) {


hfinkel wrote:
> This variable is only used inside the if statement. Why are you
> creating it out here?
It's how the others do it (BPSI is identical, which is where I copied
it from). I'll fix this one though.

================
Comment at: lib/Target/PowerPC/PPCMachineFunctionInfo.h:123
@@ -119,3 +122,3 @@

MF(MF),
UsesPICBase(0) {}

hfinkel wrote:
> We don't initialize all of the member variables here, but I'd
> rather we did. Can you please initialize PICBasePointerSaveIndex
> here.
Sure thing.

http://reviews.llvm.org/D6906

EMAIL PREFERENCES

http://reviews.llvm.org/settings/panel/emailpreferences/
jhibbits updated this revision to Diff 17958.Jan 9 2015, 3:51 PM
jhibbits edited edge metadata.

Address hfinkel's comments

hfinkel accepted this revision.Jan 9 2015, 3:57 PM
hfinkel edited edge metadata.

LGTM

This revision is now accepted and ready to land.Jan 9 2015, 3:57 PM
This revision was automatically updated to reflect the committed changes.