8000 Fix compiler warnings by fyshhh · Pull Request #51 · mpickpt/mana · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix compiler warnings #51

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

Open
wants to merge 3 commits into
base: master-old
Choose a base branch
from
Open
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
4 changes: 4 additions & 0 deletions configure
Original file line number Diff line number Diff line change
Expand Up @@ -4989,6 +4989,10 @@ if test -n "$ac_unrecognized_opts"; then
esac
fi

CFLAGS="$CFLAGS -Werror"
CPPFLAGS="$CPPFLAGS -Werror"
CXXFLAGS="$CXXFLAGS -Werror"
Comment on lines +4992 to +4994
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'd preferably like to make this possible to disable (like through a --disable-werror flag), but i'm not sure how to exclude this flag from $ac_unrecognized_opts.


# Check whether --enable-logging was given.
if test "${enable_logging+set}" = set; then :
enableval=$enable_logging; use_logging=$enableval
Expand Down
1 change: 0 additions & 1 deletion contrib/mpi-proxy-split/p2p_drain_send_recv.cpp
8000
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,6 @@ removePendingSendRequests()
for (it = g_async_calls.begin(); it != g_async_calls.end();) {
MPI_Request request = it->first;
mpi_async_call_t *call = it->second;
int flag = 0;
if (call->type == ISEND_REQUEST) {
UPDATE_REQUEST_MAP(request, MPI_REQUEST_NULL);
// FIXME: We should free `call' to avoid memory leak
Expand Down
1 change: 0 additions & 1 deletion contrib/mpi-proxy-split/p2p_log_replay.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ logRequestInfo(MPI_Request request, mpi_req_t req_type)
request_info_t*
lookupRequestInfo(MPI_Request request)
{
request_info_t *req_info;
std::unordered_map<MPI_Request, request_info_t*>::iterator it;
it = request_log.find(request);
if (it != request_log.end()) {
Expand Down
4 changes: 2 additions & 2 deletions contrib/mpi-proxy-split/split_process.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,8 @@ startProxy()
// Read from stdout of lh_proxy full lh_info struct, including orig memRange.
close(pipefd_out[1]); // close write end of pipe
// FIXME: should be a readall. Check for return error code.
if (read(pipefd_out[0], &lh_info, sizeof lh_info) < sizeof lh_info) {
if ((unsigned long) read(pipefd_out[0], &lh_info, sizeof lh_info)
< sizeof lh_info) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

read can return -1, make it unsigned long will be 2^32 - 1 and passed this condition check, which is not correct.

JWARNING(false)(JASSERT_ERRNO) .Text("Read fewer bytes than expected");
break;
}
Expand All @@ -352,7 +353,6 @@ findLHMemRange(MemRange_t *lh_mem_range)
bool is_set = false;

Area area;
char prev_path_name[PATH_MAX];
char next_path_name[PATH_MAX];
uint64_t prev_addr_end;
uint64_t next_addr_start;
Expand Down
5 changes: 2 additions & 3 deletions contrib/mpi-proxy-split/two-phase-algo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,8 @@ TwoPhaseAlgo::commit_begin(MPI_Comm comm)
// Set state again incase we returned from beforeTrivialBarrier
MPI_Request request;
MPI_Comm realComm = VIRTUAL_TO_REAL_COMM(comm);
int flag = 0;
// Consider removing the #if 0 segment below?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know what's the purpose of the #if 0 bloc below, may keep it for debugging purposes.

But in C++, we usually define the variable right before we use it, so you can move this line to 145 if we don't want to remove that #if 0 block.

// int flag = 0;
int tb_rc = -1;
JUMP_TO_LOWER_HALF(lh_info.fsaddr);
tb_rc = NEXT_FUNC(Ibarrier)(realComm, &request);
Expand Down Expand Up @@ -198,8 +199,6 @@ TwoPhaseAlgo::replayTrivialBarrier()
if (_replayTrivialBarrier) {
MPI_Request request;
MPI_Comm realComm = VIRTUAL_TO_REAL_COMM(_comm);
int flag = 0;
int tb_rc = -1;
JUMP_TO_LOWER_HALF(lh_info.fsaddr);
tb_rc = NEXT_FUNC(Ibarrier)(realComm, &request);
RETURN_TO_UPPER_HALF();
Expand Down
28 changes: 16 additions & 12 deletions src/mtcp/mtcp_restart.c
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,9 @@ int discover_union_ckpt_images(char *argv[],
return rank;
}

// This is to satisfy compiler warnings, although we can probably just remove
// it.
#if 0
NO_OPTIMIZE
static unsigned long int
mygetauxval(char **evp, unsigned long int type)
Expand All @@ -480,6 +483,7 @@ mygetauxval(char **evp, unsigned long int type)
}
return 0;
}
#endif

NO_OPTIMIZE
static int
Expand Down Expand Up @@ -512,7 +516,7 @@ uint64_t vdsoStartTmp = 0;
static void
remap_vdso_and_vvar_regions() {
Area area;
int rc = 0;
void *rc = 0;
uint64_t vvarStart = 0;
uint64_t vdsoStart = 0;
uint64_t vvarSize = 0;
Expand All @@ -527,11 +531,11 @@ remap_vdso_and_vvar_regions() {

while (mtcp_readmapsline(mapsfd, &area)) {
if (mtcp_strcmp(area.name, "[vvar]") == 0) {
vvarStart = area.addr;
vvarSize = area.size;
vvarStart = (uint64_t) area.addr;
vvarSize = (uint64_t) area.size;
} else if (mtcp_strcmp(area.name, "[vdso]") == 0) {
vdsoStart = area.addr;
vdsoSize = area.size;
vdsoStart = (uint64_t) area.addr;
vdsoSize = (uint64_t) area.size;
}

if (vvarStart > 0 && vdsoStart > 0) {
Expand All @@ -542,7 +546,7 @@ remap_vdso_and_vvar_regions() {
mtcp_sys_lseek(mapsfd, 0, SEEK_SET);

while (mtcp_readmapsline(mapsfd, &area)) {
if (prev_addr + vvarSize + vdsoSize <= area.addr) {
if (prev_addr + vvarSize + vdsoSize <= (uint64_t) area.addr) {
vvarStartTmp = prev_addr;
vdsoStartTmp = prev_addr + vvarSize;
break;
Expand Down Expand Up @@ -710,8 +714,6 @@ main(int argc, char *argv[], char **environ)
// If we want to test this, we can add code to do a trial mremap with a page
// before vvar and after vdso, and verify that we get an EFAULT.
// In May, 2020, on Cori and elsewhere, vvar is 3 pages and vdso is 2 pages.
char *vdsoStart = (char *)mygetauxval(environ, AT_SYSINFO_EHDR);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this for and why removing this line?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a currently unused variable that was previously used when determining where to temporarily map the vdso/vvar segments to. there used to be a function mysetauxval that set AT_SYSINFO_EHDR so this line could read for it, but since that function is no longer in use we can remove this one as well.

F438


remap_vdso_and_vvar_regions();

// Now that we moved vdso/vvar, we need to update the vdso address
Expand Down Expand Up @@ -773,10 +775,12 @@ main(int argc, char *argv[], char **environ)
// mtcp_restart is statically linked, and doesn't need it.
Area heap_area;
MTCP_ASSERT(getMappedArea(&heap_area, "[heap]") == 1);
start1 = max(heap_area.endAddr, lh_info.memRange.end);
start1 = (char *) max((uint64_t) heap_area.endAddr,
(uint64_t) lh_info.memRange.end);
Area stack_area;
MTCP_ASSERT(getMappedArea(&stack_area, "[stack]") == 1);
end1 = min(stack_area.endAddr - 4 * GB, highMemStart - 4 * GB);
end1 = (char *) min((uint64_t) stack_area.endAddr - 4 * GB,
(uint64_t) highMemStart - 4 * GB);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be very careful when performing minus on uint64_t, making sure the first is greater than the second.

start2 = 0;
end2 = start2;
}
Expand Down Expand Up @@ -1432,10 +1436,10 @@ unmap_memory_areas_and_restore_vdso(RestoreInfo *rinfo, LowerHalfInfo_t *lh_info
// Do not unmap lower half
DPRINTF("Skipping lower half memory section: %p-%p\n",
area.addr, area.endAddr);
} else if (area.addr == vdsoStartTmpCopy) {
} else if ((uint64_t) area.addr == vdsoStartTmpCopy) {
DPRINTF("Skipping temporary vDSO section: %p-%p\n",
area.addr, area.endAddr);
} else if (area.addr == vvarStartTmpCopy) {
} else if ((uint64_t) area.addr == vvarStartTmpCopy) {
DPRINTF("Skipping temporary vvar section: %p-%p\n",
area.addr, area.endAddr);
} else if (area.size > 0) {
Expand Down
7 changes: 3 additions & 4 deletions src/mtcp/mtcp_split_process.c
Original file line number Diff line number Diff line change
Expand Up @@ -255,17 +255,16 @@ setLhMemRange()
Area area;

const uint64_t ONE_GB = 0x40000000;
const uint64_t TWO_GB = 0x80000000;
MemRange_t lh_mem_range;

int found = getMappedArea(&area, "[stack]");
if (found) {
#if !defined(MANA_USE_LH_FIXED_ADDRESS)
lh_mem_range.start = (VA)area.addr - TWO_GB;
lh_mem_range.start = (VA)area.addr - 2 * ONE_GB;
lh_mem_range.end = (VA)area.addr - ONE_GB;
#else
lh_mem_range.start = 0x2aab00000000;
lh_mem_range.end = 0x2aab00000000 + ONE_GB;
lh_mem_range.start = (VA) 0x2aab00000000;
lh_mem_range.end = (VA) 0x2aab00000000 + ONE_GB;
#endif
} else {
DPRINTF("Failed to find [stack] memory segment\n");
Expand Down
0