LGTM too, thanks!
On Wed, Oct 31, 2012 at 10:59 AM, Edwin Vane <[email protected]> wrote: > Hi chandlerc, > > LLVM Bugzilla: http://llvm.org/bugs/show_bug.cgi?id=13221 > > This patch makes the behavior of clang consistent with the behavior of gcc > 4.6 in cases where both -fPIC and -fPIE is used. > - Separately check if -fPIE was specified in the command line and define > both __PIC__ and __PIE__ when -fPIE is used. We need to check this separately > because -fPIE will infer -fPIC even if its not explicitly used. > - Fixed existing tests. > - Added new tests for cases where both -fPIC and -fPIE is used. > > Author: Tareq A. Siraj <[email protected]> > > > http://llvm-reviews.chandlerc.com/D94 > > Files: > lib/Driver/Tools.cpp > test/Driver/pic.c > > Index: lib/Driver/Tools.cpp > =================================================================== > --- lib/Driver/Tools.cpp > +++ lib/Driver/Tools.cpp > @@ -1700,18 +1700,26 @@ > // would do to enable flag_pic. > > Arg *LastPICArg = Args.getLastArg(options::OPT_fPIC, options::OPT_fno_PIC, > - options::OPT_fpic, options::OPT_fno_pic, > - options::OPT_fPIE, options::OPT_fno_PIE, > + options::OPT_fpic, options::OPT_fno_pic); > + // We need to check for PIE flags separately because they can override the > + // PIC flags. > + Arg *LastPIEArg = Args.getLastArg(options::OPT_fPIE, options::OPT_fno_PIE, > options::OPT_fpie, options::OPT_fno_pie); > bool PICDisabled = false; > bool PICEnabled = false; > bool PICForPIE = false; > - if (LastPICArg) { > - PICForPIE = (LastPICArg->getOption().matches(options::OPT_fPIE) || > - LastPICArg->getOption().matches(options::OPT_fpie)); > + if (LastPIEArg) { > + PICForPIE = (LastPIEArg->getOption().matches(options::OPT_fPIE) || > + LastPIEArg->getOption().matches(options::OPT_fpie)); > + } > + if (LastPICArg || PICForPIE) { > + // The last PIE enabled argument can infer that PIC is enabled. > PICEnabled = (PICForPIE || > LastPICArg->getOption().matches(options::OPT_fPIC) || > LastPICArg->getOption().matches(options::OPT_fpic)); > + // We need to have PICDisabled inside the if statement because on darwin > + // PIC is enabled by default even if PIC arguments are not in the command > + // line (in this case causes PICDisabled to be true). > PICDisabled = !PICEnabled; > } > // Note that these flags are trump-cards. Regardless of the order w.r.t. > the > @@ -1746,9 +1754,13 @@ > > // Infer the __PIC__ and __PIE__ values. > if (ModelStr == "pic" && PICForPIE) { > + CmdArgs.push_back("-pic-level"); > + CmdArgs.push_back((LastPIEArg && > + LastPIEArg->getOption().matches(options::OPT_fPIE)) ? > + "2" : "1"); > CmdArgs.push_back("-pie-level"); > - CmdArgs.push_back((LastPICArg && > - LastPICArg->getOption().matches(options::OPT_fPIE)) ? > + CmdArgs.push_back((LastPIEArg && > + LastPIEArg->getOption().matches(options::OPT_fPIE)) ? > "2" : "1"); > } else if (ModelStr == "pic" || ModelStr == "dynamic-no-pic") { > CmdArgs.push_back("-pic-level"); > Index: test/Driver/pic.c > =================================================================== > --- test/Driver/pic.c > +++ test/Driver/pic.c > @@ -55,9 +55,11 @@ > // RUN: %clang -c %s -target i386-unknown-unknown -fPIE -fno-pie -### 2>&1 \ > // RUN: | FileCheck %s --check-prefix=CHECK-NO-PIC > // RUN: %clang -c %s -target i386-unknown-unknown -fpie -fno-pic -### 2>&1 \ > -// RUN: | FileCheck %s --check-prefix=CHECK-NO-PIC > +// RUN: | FileCheck %s --check-prefix=CHECK-PIC1 > +// RUN: %clang -c %s -target i386-unknown-unknown -fpie -fno-pic -### 2>&1 \ > +// RUN: | FileCheck %s --check-prefix=CHECK-PIE1 > // RUN: %clang -c %s -target i386-unknown-unknown -fpic -fno-pie -### 2>&1 \ > -// RUN: | FileCheck %s --check-prefix=CHECK-NO-PIC > +// RUN: | FileCheck %s --check-prefix=CHECK-PIC1 > // RUN: %clang -c %s -target i386-unknown-unknown -fpic -fPIC -### 2>&1 \ > // RUN: | FileCheck %s --check-prefix=CHECK-PIC2 > // RUN: %clang -c %s -target i386-unknown-unknown -fPIC -fpic -### 2>&1 \ > @@ -67,6 +69,48 @@ > // RUN: %clang -c %s -target i386-unknown-unknown -fpie -fPIC -fPIE -### > 2>&1 \ > // RUN: | FileCheck %s --check-prefix=CHECK-PIE2 > // > +// Cases where both pic and pie are specified > +// RUN: %clang -c %s -target i386-unknown-unknown -fpic -fpie -### 2>&1 \ > +// RUN: | FileCheck %s --check-prefix=CHECK-PIC1 > +// RUN: %clang -c %s -target i386-unknown-unknown -fpic -fpie -### 2>&1 \ > +// RUN: | FileCheck %s --check-prefix=CHECK-PIE1 > +// RUN: %clang -c %s -target i386-unknown-unknown -fpie -fpic -### 2>&1 \ > +// RUN: | FileCheck %s --check-prefix=CHECK-PIC1 > +// RUN: %clang -c %s -target i386-unknown-unknown -fpie -fpic -### 2>&1 \ > +// RUN: | FileCheck %s --check-prefix=CHECK-PIE1 > +// RUN: %clang -c %s -target i386-unknown-unknown -fpic -fPIE -### 2>&1 \ > +// RUN: | FileCheck %s --check-prefix=CHECK-PIC2 > +// RUN: %clang -c %s -target i386-unknown-unknown -fpic -fPIE -### 2>&1 \ > +// RUN: | FileCheck %s --check-prefix=CHECK-PIE2 > +// RUN: %clang -c %s -target i386-unknown-unknown -fpie -fPIC -### 2>&1 \ > +// RUN: | FileCheck %s --check-prefix=CHECK-PIC1 > +// RUN: %clang -c %s -target i386-unknown-unknown -fpie -fPIC -### 2>&1 \ > +// RUN: | FileCheck %s --check-prefix=CHECK-PIE1 > +// RUN: %clang -c %s -target i386-unknown-unknown -fpie -fPIE -### 2>&1 \ > +// RUN: | FileCheck %s --check-prefix=CHECK-PIC2 > +// RUN: %clang -c %s -target i386-unknown-unknown -fpie -fPIE -### 2>&1 \ > +// RUN: | FileCheck %s --check-prefix=CHECK-PIE2 > +// RUN: %clang -c %s -target i386-unknown-unknown -fPIC -fpie -### 2>&1 \ > +// RUN: | FileCheck %s --check-prefix=CHECK-PIC1 > +// RUN: %clang -c %s -target i386-unknown-unknown -fPIC -fpie -### 2>&1 \ > +// RUN: | FileCheck %s --check-prefix=CHECK-PIE1 > +// RUN: %clang -c %s -target i386-unknown-unknown -fPIC -fPIE -### 2>&1 \ > +// RUN: | FileCheck %s --check-prefix=CHECK-PIC2 > +// RUN: %clang -c %s -target i386-unknown-unknown -fPIC -fPIE -### 2>&1 \ > +// RUN: | FileCheck %s --check-prefix=CHECK-PIE2 > +// RUN: %clang -c %s -target i386-unknown-unknown -fPIE -fpic -### 2>&1 \ > +// RUN: | FileCheck %s --check-prefix=CHECK-PIC2 > +// RUN: %clang -c %s -target i386-unknown-unknown -fPIE -fpic -### 2>&1 \ > +// RUN: | FileCheck %s --check-prefix=CHECK-PIE2 > +// RUN: %clang -c %s -target i386-unknown-unknown -fPIE -fpie -### 2>&1 \ > +// RUN: | FileCheck %s --check-prefix=CHECK-PIC1 > +// RUN: %clang -c %s -target i386-unknown-unknown -fPIE -fpie -### 2>&1 \ > +// RUN: | FileCheck %s --check-prefix=CHECK-PIE1 > +// RUN: %clang -c %s -target i386-unknown-unknown -fPIE -fPIC -### 2>&1 \ > +// RUN: | FileCheck %s --check-prefix=CHECK-PIC2 > +// RUN: %clang -c %s -target i386-unknown-unknown -fPIE -fPIC -### 2>&1 \ > +// RUN: | FileCheck %s --check-prefix=CHECK-PIE2 > +// > // Defaults change for Darwin. > // RUN: %clang -c %s -target i386-apple-darwin -### 2>&1 \ > // RUN: | FileCheck %s --check-prefix=CHECK-PIC2 > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
