C passes locally, not online (Compliation Flags)

[blogscot]

I been working C grade_school and got the tests passing locally, but on submission I found that they had failed (what!!??) as follows:

./grade_school.c:10:6: error: no previous declaration for 'existing_student' [-Werror=missing-declarations]
 bool existing_student(char name[]) {
      ^~~~~~~~~~~~~~~~
./grade_school.c:21:5: error: no previous declaration for 'compare_students' [-Werror=missing-declarations]
 int compare_students(const void *a, const void *b) {
     ^~~~~~~~~~~~~~~~
./grade_school.c:28:11: error: no previous declaration for 'create_student' [-Werror=missing-declarations]
 student_t create_student(uint8_t grade, char name[]) {
           ^~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make: *** [makefile:37: tests.out] Error 1

Apparently, I need to include declarations for private functions (why?). What I don't understand why this issue isn't raised when I test locally? As far as I can see the make file includes flags for missing-declarations.

CFLAGS  = -std=c99
CFLAGS += -g
CFLAGS += -Wall
CFLAGS += -Wextra
CFLAGS += -pedantic
CFLAGS += -Werror
CFLAGS += -Wmissing-declarations
CFLAGS += -DUNITY_SUPPORT_64

-- Added --

After further investigation I learned that that clang has a -Weverything compilation flag. Running with this set I can see numerous issues with my solution. It would seem that Clang is treating the [-Werror=missing-declarations] errors reported above as a [-Wmissing-prototypes] errors.

[wolf99]

I'm not very familiar with Clang and any difference in how it treats flags.

Can you share you solution header and source file here so that I can rule out any oddities there?

I'm thinking that maybe the functions should be either declared as static, or they should have prototypes in the header file. Either way we would like to try to have the same behaviour across GNU and Clang

[ryanplusplus]

@blogscot in order to mark a function private you need to declare it as static. Without static it has global linkage even if you don't declare it in a header. Without static the compiler thinks you're trying to declare a public function but have forgotten the prototype in the header.

[wolf99]

Hi @blogscot Did this resolve the issue for you? If yes, I will close this issue. If no response I will close the issue to avoid confusion if anyone else has same issue with different root cause.

[blogscot]

@wolf99, I managed to fix the errors and warnings in my code by manually updating the makefile to include the -Weverything flag. Ideally, I think the makefile should be able to handle situations where people are working on a Mac or a PC, however, I have no idea how that might be achieved.

I still occasionally come across cases where I have a solution passing locally using Clang but it gets reported as failed once submitted online. I'm now facing the situation where my Sum of Multiples solution is working locally with no errors or warnings with the -Weverything flag added to the makefile. However, when I run the code in the Exercism editor, it reports:

make: *** [makefile:22: test] Segmentation fault (core dumped)

I'm not sure how I might go about debugging this platform incompatibility issue.

-- Update --

I found that the error was due to the way memory is being allocated on my local machine compared to online. Apparently, on my local machine I can exceed array boundaries to my heart's content with not a squeak from the compiler (I ran make memcheck which highlighted the error of my ways). I assume restrictions on memory allocation are more tightly controlled on the Exercism servers. Alas, I need to go back to the drawing board to rethink how I approach this coding problem.

To conclude, what I'd hoped was that when I got a solution working on my local machine I could confidently upload the solution to Exercism and not face any nasty surprises. However, I've found that apparently compiler flags are being interpreted differently by different compilers, and as I just described above the runtime platforms environments (local and online) probably cannot be matched exactly. So, I imagine that I'll just have to lower my expectations, with respect to developing with C anyway.

[wolf99]

Hi @blogscot,

I'm not sure why the compiler operation would differ locally to online. The online runner uses the same makefile as gets downloaded when running locally. The runner uses Alpine 3.10 as the OS, so perhaps the compiler runs a little differently there. What environment are you building on locally?

[blogscot]

Thanks for the info @wolf99,

I'm running on macOS Big Sur 11.6.1, using VS Code 1.62.1. The compiler details are:

Apple clang version 12.0.5 (clang-1205.0.22.9)
Target: x86_64-apple-darwin20.6.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

I'll set up a docker container locally with the alpine image you mentioned and experiment by recreating my coding errors to see how they are handled by the Linux compiler. My preference is find faults in my code as early as possible, not when I upload my solutions. If that means running in a docker container I'd be happy with that outcome.

[wolf99]

Great! I don't have Mac env to be able to test on. I'm surprised theres a difference though, I would have expected Windows to be a culprit! Looking forward to your results in this regard.

If you're happy to develop in a docker container that's good, but I think it is not the best user experience. Perhaps we can have some issue raised for this on the C track, what do you think @ryanplusplus ?

BTW, I have a PR raised to up date the Alpine version to the most recent but it has some test issues to resolve before merging.

[blogscot]

I setup my environment to use a ubuntu:18.04 docker imagine:

FROM ubuntu:18.04

RUN apt-get update && apt-get -y --no-install-recommends install \
  build-essential \
  vim

COPY . /usr/src/myapp
WORKDIR /usr/src/myapp

LABEL Name=sumofmultiples Version=0.0.1

I was able to recreate my original problem where Clang appears to treat gcc missing-declarations errors as missing-prototypes errors.

On the other hand, the error where I allocated space for 50 ints but unintentionally used 8K instead, works just fine both locally and in my docker container. I am curious how the Exercism server tests catches this type of error (I may explore the official dockerfile to glean what I can).

I learned that hard way that I should have run both make test and make memcheck to catch my mistake. There are probably good reasons why these tests are separated, but could they be combined?

[wolf99]

Great to see that the missing-declarations issue can be reproduced! This issue can continue to deal with that item.

WRT the memcheck target in the makefle, it is separated from the test target because of the expectation that interpreting the output of memcheck would be quite complex for less experienced students. There have been some discussions on adding memcheck to the default tests, so far resulting in the decision to leave it out/leave it to the analyzer (which does not exist yet).

We would welcome your input as a student on the open issues around this

• https://github.com/exercism/c-test-runner/issues/10
• https://github.com/exercism/c-test-runner/issues/11

I suggest not to continue that discussion on this issue though, so as not to confuse the matters.

[wolf99]

Have seen the following in my environment (Ubuntu 20.04)

# default to gcc
$ unset CC

# Run make with `static` missing from one of the functions in the grade_school.c file
$  make clean
rm -rf *.o *.out *.out.dSYM

$ make
Compiling tests.out
./grade_school.c:5:6: error: no previous declaration for ‘student_is_on_roster’ [-Werror=missing-declarations]
    5 | bool student_is_on_roster(roster_t * roster, char *name)
      |      ^~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make: *** [makefile:37: tests.out] Error 1

$ make clean
rm -rf *.o *.out *.out.dSYM

# Switch to using clang
$ export CC=/usr/bin/clang

$  make
Compiling tests.out
test_grade_school.c:346:test_roster_is_empty_when_no_student_added:PASS
test_grade_school.c:347:test_add_student:PASS

#... everything runs as if correct 😨 

If I add CLFAGS += -Wmissing-prototypes to the makefile then compiling with CC set to clang gives the expected failure:

$ export CC=/usr/bin/clang

$ make
Compiling tests.out
./grade_school.c:5:6: error: no previous prototype for function 'student_is_on_roster' [-Werror,-Wmissing-prototypes]
bool student_is_on_roster(roster_t * roster, char *name)
     ^
./grade_school.c:5:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
bool student_is_on_roster(roster_t * roster, char *name)
^
static
/usr/lib/llvm-10/lib/clang/10.0.0/include/stdbool.h:15:14: note: expanded from macro 'bool'
#define bool _Bool
             ^
1 error generated.
make: *** [makefile:38: tests.out] Error 1

# GCC gives only a slightly different error than previously
$ unset CC

$ make clean
rm -rf *.o *.out *.out.dSYM

$ make
Compiling tests.out
./grade_school.c:5:6: error: no previous prototype for ‘student_is_on_roster’ [-Werror=missing-prototypes]
    5 | bool student_is_on_roster(roster_t * roster, char *name)
      |      ^~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make: *** [makefile:38: tests.out] Error 1

Given this, I propose that we add CLFAGS += -Wmissing-prototypes to all makefiles in the track. What do you think @ryanplusplus , do you see any down side?

[blogscot]

In my last comment I raised the possibility of having make memcheck become a default part of the testing process. Immediately after suggesting that, I worked on the Bob exercise; had it working on both my machine, and using Docker only to find that it failed on Exercism - I had only run the tests using make test again! After some trial and error, I found I hadn't allocated enough memory to terminate a string correctly. If only I had run memcheck I might have picked up the bug earlier, I thought to myself.

To confirm this I retested my original solution with make memcheck only to discover that I had a second quite serious memory bug in my code that wasn't caught either locally, using Docker, nor running on Exercism.

In the code below, I had missed using pos terminate the loop! For blank strings the array access would have ended up in a distant part of memory, most of the time returning false, which would be the correct behaviour most of the time.

static bool isquestion(char *greeting) {
  size_t pos = strlen(greeting) - 1;
  while (pos > 0 && isspace(greeting[pos])) {
    pos--;
  }
  return greeting[pos] == '?';
}

This is to highlight how important it is to run the memory checks, and how easy it is to be lulled into thinking that your code in working when make test tells you that everything is passing. Hence, my proposal is to modify the makefile to include something similar to the following:

.PHONY: all
all: clean memcheck

I believe because it's quicker and easier to type make all than the alternatives most people will tend to choose this option, however, if people prefer it's still possible to run tests as before.

[Anomalocaridid]

In regards to what @wolf99 said:

I'm not sure why the compiler operation would differ locally to online.

After I re-download an exercise where I have this issue, it looks like the tests update and running them gives the same issue as the online tests result. The exercises that I re-downloaded are Pythagorean triple and beer song. My PC runs the Sway edition of the EndeavourOS Linux distro.

Edit: My problem was actually caused by the code files being in the wrong locations. They changed after I initially submitted my solutions.

[joshgoebel]

make: *** [makefile:22: test] Segmentation fault (core dumped)

I saw this often when trying to get started the C track online. So much so that I think something is broken in the test runner. Even if the compiled binary were the segfault we should still get a clear indication that that was the case (where-as now [to me] it looked like more of an infrastructure issue). Also, wouldn't most of these sorts of issues be prevented if we did make memcheck in the runner?