From 6d820037e92988197e10264a810ebdb4f2b8aebc Mon Sep 17 00:00:00 2001 From: Austen Lauria Date: Wed, 27 Jan 2021 15:51:33 -0500 Subject: [PATCH] Make sure MPIR_Breakpoint() is compiled without CFLAGS. In optimized builds, CFLAGS contains various optimizations such as -O3, and is propogated by automake to all files. To work-around this, isolate MPIR_Breakpoint() and other MPIR_* symbols into its own library built with debugger specific CFLAGS. To prevent CFLAGS from being polluted elsewhere in the make tree, build this in its own tiny stand-alone makefile. Fixes #7757 Signed-off-by: Austen Lauria --- config/orte_config_files.m4 | 3 +- orte/Makefile.am | 6 +- orte/orted/Makefile.am | 14 +--- orte/orted/orted-mpir/Makefile.am | 22 +++++++ orte/orted/orted-mpir/orted_mpir.h | 36 ++++++++++ orte/orted/orted-mpir/orted_mpir_breakpoint.c | 65 +++++++++++++++++++ orte/orted/orted_submit.c | 58 +---------------- 7 files changed, 134 insertions(+), 70 deletions(-) create mode 100644 orte/orted/orted-mpir/Makefile.am create mode 100644 orte/orted/orted-mpir/orted_mpir.h create mode 100644 orte/orted/orted-mpir/orted_mpir_breakpoint.c diff --git a/config/orte_config_files.m4 b/config/orte_config_files.m4 index 191d280131c..16c7ee1103c 100644 --- a/config/orte_config_files.m4 +++ b/config/orte_config_files.m4 @@ -7,6 +7,7 @@ # Copyright (c) 2011-2012 Los Alamos National Security, LLC. All rights # reserved. # Copyright (c) 2015-2018 Intel, Inc. All rights reserved. +# Copyright (c) 2021 IBM Corporation. All rights reserved. # $COPYRIGHT$ # # Additional copyrights may follow @@ -19,7 +20,7 @@ AC_DEFUN([ORTE_CONFIG_FILES],[ orte/Makefile orte/include/Makefile orte/etc/Makefile - + orte/orted/orted-mpir/Makefile orte/tools/orted/Makefile orte/tools/orterun/Makefile orte/tools/wrappers/Makefile diff --git a/orte/Makefile.am b/orte/Makefile.am index 6af81a22e39..30cecee1088 100644 --- a/orte/Makefile.am +++ b/orte/Makefile.am @@ -12,6 +12,7 @@ # Copyright (c) 2009-2014 Cisco Systems, Inc. All rights reserved. # Copyright (c) 2015 Los Alamos National Security, LLC. All rights # reserved. +# Copyright (c) 2021 IBM Corporation. All rights reserved. # $COPYRIGHT$ # # Additional copyrights may follow @@ -24,12 +25,14 @@ SUBDIRS = \ $(MCA_orte_FRAMEWORKS_SUBDIRS) \ $(MCA_orte_FRAMEWORK_COMPONENT_STATIC_SUBDIRS) \ etc \ + orted/orted-mpir \ . \ $(MCA_orte_FRAMEWORK_COMPONENT_DSO_SUBDIRS) DIST_SUBDIRS = \ include \ etc \ + orted/orted-mpir \ $(MCA_orte_FRAMEWORKS_SUBDIRS) \ $(MCA_orte_FRAMEWORK_COMPONENT_ALL_SUBDIRS) @@ -39,7 +42,8 @@ lib_LTLIBRARIES = lib@ORTE_LIB_PREFIX@open-rte.la lib@ORTE_LIB_PREFIX@open_rte_la_SOURCES = lib@ORTE_LIB_PREFIX@open_rte_la_LIBADD = \ $(MCA_orte_FRAMEWORK_LIBS) \ - $(ORTE_TOP_BUILDDIR)/opal/lib@OPAL_LIB_PREFIX@open-pal.la + $(ORTE_TOP_BUILDDIR)/opal/lib@OPAL_LIB_PREFIX@open-pal.la \ + orted/orted-mpir/lib@ORTE_LIB_PREFIX@open-orted-mpir.la lib@ORTE_LIB_PREFIX@open_rte_la_DEPENDENCIES = $(libopen_rte_la_LIBADD) lib@ORTE_LIB_PREFIX@open_rte_la_LDFLAGS = -version-info $(libopen_rte_so_version) diff --git a/orte/orted/Makefile.am b/orte/orted/Makefile.am index 1235e51e69b..c708e2b01ed 100644 --- a/orte/orted/Makefile.am +++ b/orte/orted/Makefile.am @@ -12,7 +12,7 @@ # All rights reserved. # Copyright (c) 2014 Cisco Systems, Inc. All rights reserved. # Copyright (c) 2015 Intel, Inc. All rights reserved. -# Copyright (c) 2018 IBM Corporation. All rights reserved. +# Copyright (c) 2018-2021 IBM Corporation. All rights reserved. # $COPYRIGHT$ # # Additional copyrights may follow @@ -30,17 +30,7 @@ headers += \ lib@ORTE_LIB_PREFIX@open_rte_la_SOURCES += \ orted/orted_main.c \ - orted/orted_comm.c - -# The MPIR portion of the library must be built with -g, even if -# the rest of the library has other optimization flags. -# Use an intermediate library to isolate the debug object. -noinst_LTLIBRARIES += liborted_mpir.la -liborted_mpir_la_SOURCES = \ + orted/orted_comm.c \ orted/orted_submit.c -liborted_mpir_la_CFLAGS = $(CFLAGS_WITHOUT_OPTFLAGS) $(DEBUGGER_CFLAGS) - -lib@ORTE_LIB_PREFIX@open_rte_la_LIBADD += liborted_mpir.la - include orted/pmix/Makefile.am diff --git a/orte/orted/orted-mpir/Makefile.am b/orte/orted/orted-mpir/Makefile.am new file mode 100644 index 00000000000..5c0dd335644 --- /dev/null +++ b/orte/orted/orted-mpir/Makefile.am @@ -0,0 +1,22 @@ +# -*- makefile -*- +# +# Copyright (c) 2021 IBM Corporation. All rights reserved. +# $COPYRIGHT$ +# +# Additional copyrights may follow +# +# $HEADER$ +# + +# This is not quite in the Automake spirit, but we have to do it. +# Since the mpir portion of the library must be built with -g, we +# must eliminate the CFLAGS that are passed in here by default (which +# may already have debugging and/or optimization flags). + +CFLAGS = $(CFLAGS_WITHOUT_OPTFLAGS) $(DEBUGGER_CFLAGS) + +lib_LTLIBRARIES = lib@ORTE_LIB_PREFIX@open-orted-mpir.la +lib@ORTE_LIB_PREFIX@open_orted_mpir_la_SOURCES = \ + orted_mpir_breakpoint.c \ + orted_mpir.h +lib@ORTE_LIB_PREFIX@open_orted_mpir_la_LDFLAGS = -avoid-version diff --git a/orte/orted/orted-mpir/orted_mpir.h b/orte/orted/orted-mpir/orted_mpir.h new file mode 100644 index 00000000000..a4ed7d08620 --- /dev/null +++ b/orte/orted/orted-mpir/orted_mpir.h @@ -0,0 +1,36 @@ +/* Copyright (c) 2021 IBM Corporation. All rights reserved. + * $COPYRIGHT$ + * + * Additional copyrights may follow + * + * $HEADER$ + */ + +#ifndef ORTED_MPIR_H +#define ORTED_MPIR_H + +#include "orte_config.h" + +#include "orte/runtime/orte_globals.h" + +BEGIN_C_DECLS + +#define MPIR_MAX_PATH_LENGTH 512 +#define MPIR_MAX_ARG_LENGTH 1024 + +extern struct MPIR_PROCDESC *MPIR_proctable; +extern int MPIR_proctable_size; +extern volatile int MPIR_being_debugged; +extern volatile int MPIR_debug_state; +extern int MPIR_i_am_starter; +extern int MPIR_partial_attach_ok; +extern char MPIR_executable_path[MPIR_MAX_PATH_LENGTH]; +extern char MPIR_server_arguments[MPIR_MAX_ARG_LENGTH]; +extern volatile int MPIR_forward_output; +extern volatile int MPIR_forward_comm; +extern char MPIR_attach_fifo[MPIR_MAX_PATH_LENGTH]; +extern int MPIR_force_to_main; + +ORTE_DECLSPEC void __opal_attribute_optnone__ MPIR_Breakpoint(void); + +#endif diff --git a/orte/orted/orted-mpir/orted_mpir_breakpoint.c b/orte/orted/orted-mpir/orted_mpir_breakpoint.c new file mode 100644 index 00000000000..e061c59a9df --- /dev/null +++ b/orte/orted/orted-mpir/orted_mpir_breakpoint.c @@ -0,0 +1,65 @@ +/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */ +/* + * Copyright (c) 2021 IBM Corporation. All rights reserved. + * $COPYRIGHT$ + * + * Additional copyrights may follow + * + * $HEADER$ + */ + +#include "orte_config.h" +#include "orte/constants.h" +#include "orted_mpir.h" + +/* instance the standard MPIR interfaces */ +struct MPIR_PROCDESC *MPIR_proctable = NULL; +int MPIR_proctable_size = 0; +volatile int MPIR_being_debugged = 0; +volatile int MPIR_debug_state = 0; +int MPIR_i_am_starter = 0; +int MPIR_partial_attach_ok = 1; +char MPIR_executable_path[MPIR_MAX_PATH_LENGTH] = {0}; +char MPIR_server_arguments[MPIR_MAX_ARG_LENGTH] = {0}; +volatile int MPIR_forward_output = 0; +volatile int MPIR_forward_comm = 0; +char MPIR_attach_fifo[MPIR_MAX_PATH_LENGTH] = {0}; +int MPIR_force_to_main = 0; + +/* + * Attempt to prevent the compiler from optimizing out + * MPIR_Breakpoint(). + * + * Some older versions of automake can add -O3 to every + * file via CFLAGS (which was demonstrated in automake v1.13.4), + * so there is a possibility that the compiler will see + * this function as a NOOP and optimize it out on older versions. + * While using the current/recommended version of automake + * does not do this, the following will help those + * stuck with an older version, as well as guard against + * future regressions. + * + * See the following git issue for more discussion: + * https://github.com/open-mpi/ompi/issues/5501 + */ +volatile void* volatile orte_noop_mpir_breakpoint_ptr = NULL; + +/* + * Breakpoint function for parallel debuggers + */ +void MPIR_Breakpoint(void) +{ + /* + * Actually do something with this pointer to make + * sure the compiler does not optimize out this function. + * The compiler should be forced to keep this + * function around due to the volatile void* type. + * + * This pointer doesn't actually do anything other than + * prevent unwanted optimization, and + * *should not* be used anywhere else in the code. + * So pointing this to the weeds should be OK. + */ + orte_noop_mpir_breakpoint_ptr = (volatile void *) 0x42; + return; +} diff --git a/orte/orted/orted_submit.c b/orte/orted/orted_submit.c index 0db2703e46d..53f2ce139f2 100644 --- a/orte/orted/orted_submit.c +++ b/orte/orted/orted_submit.c @@ -17,7 +17,7 @@ * Copyright (c) 2013-2018 Intel, Inc. All rights reserved. * Copyright (c) 2015-2018 Research Organization for Information Science * and Technology (RIST). All rights reserved. - * Copyright (c) 2017 IBM Corporation. All rights reserved. + * Copyright (c) 2017-2021 IBM Corporation. All rights reserved. * $COPYRIGHT$ * * Additional copyrights may follow @@ -108,6 +108,7 @@ #include "orte/util/show_help.h" #include "orted_submit.h" +#include "orted-mpir/orted_mpir.h" /** * Global struct for catching orte command line options. @@ -156,63 +157,8 @@ static void run_debugger(char *basename, opal_cmd_line_t *cmd_line, int argc, char *argv[], int num_procs); static void print_help(void); -/* instance the standard MPIR interfaces */ -#define MPIR_MAX_PATH_LENGTH 512 -#define MPIR_MAX_ARG_LENGTH 1024 -struct MPIR_PROCDESC *MPIR_proctable = NULL; -int MPIR_proctable_size = 0; -volatile int MPIR_being_debugged = 0; -volatile int MPIR_debug_state = 0; -int MPIR_i_am_starter = 0; -int MPIR_partial_attach_ok = 1; -char MPIR_executable_path[MPIR_MAX_PATH_LENGTH] = {0}; -char MPIR_server_arguments[MPIR_MAX_ARG_LENGTH] = {0}; -volatile int MPIR_forward_output = 0; -volatile int MPIR_forward_comm = 0; -char MPIR_attach_fifo[MPIR_MAX_PATH_LENGTH] = {0}; -int MPIR_force_to_main = 0; static void orte_debugger_init_before_spawn(orte_job_t *jdata); -ORTE_DECLSPEC void __opal_attribute_optnone__ MPIR_Breakpoint(void); - -/* - * Attempt to prevent the compiler from optimizing out - * MPIR_Breakpoint(). - * - * Some older versions of automake can add -O3 to every - * file via CFLAGS (which was demonstrated in automake v1.13.4), - * so there is a possibility that the compiler will see - * this function as a NOOP and optimize it out on older versions. - * While using the current/recommended version of automake - * does not do this, the following will help those - * stuck with an older version, as well as guard against - * future regressions. - * - * See the following git issue for more discussion: - * https://github.com/open-mpi/ompi/issues/5501 - */ -volatile void* volatile orte_noop_mpir_breakpoint_ptr = NULL; - -/* - * Breakpoint function for parallel debuggers - */ -void MPIR_Breakpoint(void) -{ - /* - * Actually do something with this pointer to make - * sure the compiler does not optimize out this function. - * The compiler should be forced to keep this - * function around due to the volatile void* type. - * - * This pointer doesn't actually do anything other than - * prevent unwanted optimization, and - * *should not* be used anywhere else in the code. - * So pointing this to the weeds should be OK. - */ - orte_noop_mpir_breakpoint_ptr = (volatile void *) 0x42; - return; -} - /* local objects */ typedef struct { opal_object_t super;