The attached patch adds error messages to be emitted when -mcode/data-region are
used without the accompanying -mlarge option that enables the large memory
model.
Use of the upper/either regions without -mlarge can cause relocation overflows
or stack mismanagement when incorrect call/return instructions are generated.
In most cases, using the lower region without -mlarge has no effect, but it can
affect code generation unexpectedly (msp430_do_not_relax_short_jumps), or
add the ".lower" prefix to some section names.
Similarly, handling of the upper/lower/either attributes has been
modified so the attributes will be ignored, and a warning will be emitted, when
they are used without -mlarge.
I have added the error messages to the driver so that uses of
-mcode/data-region=none can be caught. The compiler cannot catch "none" since
it is the default value that the option variable is initialized to.
So only users calling cc1* directly will see the errors in
msp430_option_override, but I also have updated them to be more similar to the
new errors emitted by the driver.
Successfully regtested on trunk for msp430-elf.
Ok for trunk?
>From 0c37dc30d67229392ae8f834e462dd6336f7ccfc Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <joze...@mittosystems.com>
Date: Mon, 22 Jul 2019 21:37:45 +0100
Subject: [PATCH] MSP430: Disallow mcode/data-region unless the large memory
model is in use
gcc/ChangeLog:
2019-07-23 Jozef Lawrynowicz <joze...@mittosystems.com>
* config/msp430/msp430.h (DRIVER_SELF_SPECS): Define and emit errors
when -m{code,data}-region are used without -mlarge.
* config/msp430/msp430.c (msp430_option_override): Error when a
non-default code or data region is used without -mlarge.
(msp430_section_attr): Emit a warning and do not add upper/lower/either
attributes when they are used without -mlarge.
gcc/testsuite/ChangeLog:
2019-07-23 Jozef Lawrynowicz <joze...@mittosystems.com>
* gcc.target/msp430/pr78818-data-region.c: Add -mlarge to dg-options.
* gcc.target/msp430/region-misuse-code.c: New test.
* gcc.target/msp430/region-misuse-data.c: Likewise.
* gcc.target/msp430/region-misuse-code-data.c: Likewise.
* gcc.target/msp430/region-attribute-misuse.c: Likewise.
---
gcc/config/msp430/msp430.c | 35 ++++++++++++++++---
gcc/config/msp430/msp430.h | 8 +++++
.../gcc.target/msp430/pr78818-data-region.c | 3 +-
.../msp430/region-attribute-misuse.c | 16 +++++++++
.../msp430/region-misuse-code-data.c | 4 +++
.../gcc.target/msp430/region-misuse-code.c | 4 +++
.../gcc.target/msp430/region-misuse-data.c | 4 +++
7 files changed, 69 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/msp430/region-attribute-misuse.c
create mode 100644 gcc/testsuite/gcc.target/msp430/region-misuse-code-data.c
create mode 100644 gcc/testsuite/gcc.target/msp430/region-misuse-code.c
create mode 100644 gcc/testsuite/gcc.target/msp430/region-misuse-data.c
diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c
index 265c2f642d8..c7b774e71a6 100644
--- a/gcc/config/msp430/msp430.c
+++ b/gcc/config/msp430/msp430.c
@@ -875,10 +875,26 @@ msp430_option_override (void)
if (TARGET_LARGE && !msp430x)
error ("%<-mlarge%> requires a 430X-compatible %<-mmcu=%>");
- if (msp430_code_region == MSP430_REGION_UPPER && ! msp430x)
- error ("%<-mcode-region=upper%> requires 430X-compatible cpu");
- if (msp430_data_region == MSP430_REGION_UPPER && ! msp430x)
- error ("%<-mdata-region=upper%> requires 430X-compatible cpu");
+ if (!TARGET_LARGE && msp430_code_region == MSP430_REGION_EITHER)
+ error ("%<-mcode-region=either%> requires the large memory model "
+ "(%<-mlarge%>)");
+ else if (!TARGET_LARGE && msp430_code_region == MSP430_REGION_UPPER)
+ error ("%<-mcode-region=upper%> requires the large memory model "
+ "(%<-mlarge%>)");
+ else if (!TARGET_LARGE && msp430_code_region == MSP430_REGION_LOWER)
+ error ("%<-mcode-region=lower%> requires the large memory model "
+ "(%<-mlarge%>)");
+
+ if (!TARGET_LARGE && msp430_data_region == MSP430_REGION_EITHER)
+ error ("%<-mdata-region=either%> requires the large memory model "
+ "(%<-mlarge%>)");
+ else if (!TARGET_LARGE && msp430_data_region == MSP430_REGION_UPPER)
+ error ("%<-mdata-region=upper%> requires the large memory model "
+ "(%<-mlarge%>)");
+ else if (!TARGET_LARGE && msp430_data_region == MSP430_REGION_LOWER)
+ error ("%<-mdata-region=lower%> requires the large memory model "
+ "(%<-mlarge%>)");
+
if (flag_exceptions || flag_non_call_exceptions
|| flag_unwind_tables || flag_asynchronous_unwind_tables)
@@ -2038,6 +2054,17 @@ msp430_section_attr (tree * node,
message = "already marked with 'upper' attribute";
}
+ /* It does not make sense to use upper/lower/either attributes without
+ -mlarge.
+ Without -mlarge, "lower" is the default and only region, so is redundant.
+ Without -mlarge, "upper" will (and "either" might) place code/data in the
+ upper region, which for data could result in relocation overflows, and for
+ code could result in stack mismanagement and incorrect call/return
+ instructions. */
+ if (!TARGET_LARGE)
+ message = G_("%qE attribute ignored. large memory model (%<-mlarge%>) "
+ "is required");
+
if (message)
{
warning (OPT_Wattributes, message, name);
diff --git a/gcc/config/msp430/msp430.h b/gcc/config/msp430/msp430.h
index 1288b1a263d..8d2354824a0 100644
--- a/gcc/config/msp430/msp430.h
+++ b/gcc/config/msp430/msp430.h
@@ -67,6 +67,14 @@ extern bool msp430x;
#define LINK_SPEC "%{mrelax:--relax} %{mlarge:%{!r:%{!g:--gc-sections}}} " \
"%{mcode-region=*:--code-region=%*} %{mdata-region=*:--data-region=%*}"
+#define DRIVER_SELF_SPECS \
+ " %{!mlarge:%{mcode-region=*:%{mdata-region=*:%e-mcode-region and " \
+ "-mdata-region require the large memory model (-mlarge)}}}" \
+ " %{!mlarge:%{mcode-region=*:" \
+ "%e-mcode-region requires the large memory model (-mlarge)}}" \
+ " %{!mlarge:%{mdata-region=*:" \
+ "%e-mdata-region requires the large memory model (-mlarge)}}"
+
extern const char * msp430_select_hwmult_lib (int, const char **);
# define EXTRA_SPEC_FUNCTIONS \
{ "msp430_hwmult_lib", msp430_select_hwmult_lib },
diff --git a/gcc/testsuite/gcc.target/msp430/pr78818-data-region.c b/gcc/testsuite/gcc.target/msp430/pr78818-data-region.c
index 3244c0ad85d..5b0721e728a 100644
--- a/gcc/testsuite/gcc.target/msp430/pr78818-data-region.c
+++ b/gcc/testsuite/gcc.target/msp430/pr78818-data-region.c
@@ -1,5 +1,6 @@
/* { dg-do compile } */
-/* { dg-options "-mdata-region=either" } */
+/* { dg-skip-if "" { *-*-* } { "-mcpu=msp430" } { "" } } */
+/* { dg-options "-mlarge -mdata-region=either" } */
/* { dg-final { scan-assembler-not "\\.either\\.data" } } */
/* { dg-final { scan-assembler-not "\\.either\\.bss" } } */
diff --git a/gcc/testsuite/gcc.target/msp430/region-attribute-misuse.c b/gcc/testsuite/gcc.target/msp430/region-attribute-misuse.c
new file mode 100644
index 00000000000..fe4617b917c
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/region-attribute-misuse.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-mcpu=msp430" "-mlarge" "-mcode-region=*" "-mdata-region=*" } { "" } } */
+/* { dg-final { scan-assembler-not ".section.*bss" } } */
+/* { dg-final { scan-assembler ".section.*upper.data" } } */
+/* { dg-final { scan-assembler ".section.*lower.data" } } */
+/* { dg-final { scan-assembler ".section.*either.data" } } */
+
+int __attribute__((upper)) upper_bss; /* { dg-warning "'upper' attribute ignored. large memory model .'-mlarge'. is required" } */
+int __attribute__((lower)) lower_bss; /* { dg-warning "'lower' attribute ignored. large memory model .'-mlarge'. is required" } */
+int __attribute__((either)) either_bss; /* { dg-warning "'either' attribute ignored. large memory model .'-mlarge'. is required" } */
+
+/* Verify that even without -mlarge, objects can still be placed in
+ upper/lower/either regions manually. */
+int __attribute__((section(".upper.data"))) upper_data = 1;
+int __attribute__((section(".lower.data"))) lower_data = 2;
+int __attribute__((section(".either.data"))) either_data = 3;
diff --git a/gcc/testsuite/gcc.target/msp430/region-misuse-code-data.c b/gcc/testsuite/gcc.target/msp430/region-misuse-code-data.c
new file mode 100644
index 00000000000..72a160d0bda
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/region-misuse-code-data.c
@@ -0,0 +1,4 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-mlarge" } { "" } } */
+/* { dg-options "-mcode-region=either -mdata-region=none" } */
+/* { dg-error "-mcode-region and -mdata-region require the large memory model .-mlarge." "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/gcc.target/msp430/region-misuse-code.c b/gcc/testsuite/gcc.target/msp430/region-misuse-code.c
new file mode 100644
index 00000000000..6441b773d6e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/region-misuse-code.c
@@ -0,0 +1,4 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-mlarge" "-mdata-region=*" } { "" } } */
+/* { dg-options "-mcode-region=lower" } */
+/* { dg-error "-mcode-region requires the large memory model .-mlarge." "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/gcc.target/msp430/region-misuse-data.c b/gcc/testsuite/gcc.target/msp430/region-misuse-data.c
new file mode 100644
index 00000000000..07523b6042d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/msp430/region-misuse-data.c
@@ -0,0 +1,4 @@
+/* { dg-do compile } */
+/* { dg-skip-if "" { *-*-* } { "-mlarge" "-mdata-region=*" } { "" } } */
+/* { dg-options "-mdata-region=upper" } */
+/* { dg-error "-mdata-region requires the large memory model .-mlarge." "" { target *-*-* } 0 } */
--
2.17.1
