Skip to content

v3.1.x: Make sure MPIR_Breakpoint() is compiled without CFLAGS. #8432

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion config/orte_config_files.m4
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
# Copyright (c) 2011-2012 Los Alamos National Security, LLC. All rights
# reserved.
# Copyright (c) 2015-2017 Intel, Inc. All rights reserved.
# Copyright (c) 2021 IBM Corporation. All rights reserved.
# $COPYRIGHT$
#
# Additional copyrights may follow
Expand All @@ -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
Expand Down
6 changes: 5 additions & 1 deletion orte/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)

Expand All @@ -39,7 +42,8 @@ lib_LTLIBRARIES = lib@[email protected]
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@[email protected]
$(ORTE_TOP_BUILDDIR)/opal/lib@[email protected] \
orted/orted-mpir/lib@[email protected]
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)

Expand Down
14 changes: 2 additions & 12 deletions orte/orted/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
22 changes: 22 additions & 0 deletions orte/orted/orted-mpir/Makefile.am
Original file line number Diff line number Diff line change
@@ -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@[email protected]
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
36 changes: 36 additions & 0 deletions orte/orted/orted-mpir/orted_mpir.h
Original file line number Diff line number Diff line change
@@ -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
65 changes: 65 additions & 0 deletions orte/orted/orted-mpir/orted_mpir_breakpoint.c
Original file line number Diff line number Diff line change
@@ -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;
}
58 changes: 2 additions & 56 deletions orte/orted/orted_submit.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
* Copyright (c) 2013-2018 Intel, Inc. All rights reserved.
* Copyright (c) 2015-2017 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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand Down