Skip to content

Add csafe and cppsafe executables with sanitizers support #1878

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

Closed
wants to merge 2 commits into from

Conversation

rmartinsanta
Copy link

@rmartinsanta rmartinsanta commented Jan 18, 2023

We are using a modified version of the C executable in our introductory programming course. In contrast to other programming languages, C has a lot of behavior that is undefined but does not throw an immediate error. For example, it does not check if memory accesses are valid, as it is considered the responsibility of the user. Consider the following C code:

#include<stdio.h>
#define max(a,b)	(a<b? b:a)
long long int a[100001]={0},n,i,c=0,d=0,t,x;
int main(){
	scanf("%I64d",&n);
	for(i=0;i<n;i++){
		scanf("%I64d",&x);
		a[x]+=x;
	}
	for(i=1;i<=100001;i++){
		t=c;
		c=max(c,d+a[i]),d=t;
	}
	printf("%I64d",c);
	return 0;
}

While it may be fit for purpose in a competitive programming context, it clearly contains an out of bounds access to the array, due to looping i<=100001 instead of i<100001. The instructor may want to automatically detect and consider this kind of mistakes as not correct, and may want to force the user to fix the bug. Example in our Domjudge instance changing the C executable for the Csafe version, changes the veredict from correct to runtime error, and prints the line where the invalid access was produced.

image

Additional notes:

  • We do not want to replace the current C and cpp executables, we would like to allow Domjudge administrators to decide which behavior is preferable for their needs.
    -static flag was removed from the script as it was incompatible with some sanitizer checks and I am not sure why it is necessary.
  • As we have manually configured it in our Domjudge instance as an executable, we do not know if the changes included in this PR are enough to make it work by default for new Domjudge instances, or even if the folder structure is correct. Do not hesitate to correct us
  • An idea for further improvement may be creating custom error codes for different sanitizer failures: right now all fall under Runtime Error

Any suggestion or feedback is welcome. Thanks @isaaclo97 for the help testing and validating the idea.

@vmcj
Copy link
Member

vmcj commented Jan 18, 2023

I can see the use case, but what is the reasoning to not add these comments to the main language. An administrator can change the executable and add these options already. In the current form this is duplication and the admin can already change these options via the UI whereas this PR still needs a change to the C/C++ language.

@isaaclo97
Copy link

Hi @vmcj and thanks for your reply.
We besides using DomJudge as a great system to manage competitive programming contests, we use the environment also in the educational environment. We developed different basic problems oriented to teaching, many of the teachers do not spend much time to research about a certain tool.
This proposal is suitable for teachers who want to use in an educational environment and as a "strict validator" in competitive programming contests, the tests in fact have been performed on a problem already published on the Codeforces platform on all possible solutions with AC and have been obtained several wrong submissions (hackable), a student in an academic way should be able to identify this error and avoid it.
The proposal of the PR is to make it easier for a teacher to have this form of compilation to use in their teaching tasks (we have been using it years ago), to report more feedback to their students and get more consolidated knowledge.
If you consider it is not relevant, we will create a new PR as stated in your comment, unfortunately many of the teachers will not investigate the different ways to add these executables if we add it as a comment they will not see it, since they will not search inside (in our experience).

@eldering
Copy link
Member

I agree with @vmcj: I can see the use case for this in a teaching environment, but I'd rather not add this as an extra language to the main DOMjudge codebase. In the past we've had similar requests to e.g. add a version of the C or C++ compile scripts that link against specific libraries, such as libgmp. If we add these, then we'd slowly end up with a large collection of compile scripts for the same languages, all with slight variations, and we could even get into questions of creating possible combinations of these.
I think instead that it is better to leave it up to the administrator of an installation to add/modify versions of the compile scripts that they require. Various useful options to consider and how to do so could for example be added to https://github.com/DOMjudge/domjudge/wiki/Adding-a-new-language or a similar page, and we could clearly reference that from the admin manual.

@vmcj
Copy link
Member

vmcj commented Mar 1, 2023

Hi @rmartinsanta & @isaaclo97,

I've created https://github.com/DOMjudge/domjudge/wiki/DOMjudge-in-education which I'll link from the administrator manual.

As explained by @eldering we don't see a proper solution yet to fully incorporate your change.

I'll close this PR for now, but feel free to discuss what works best to promote these usecases to other educators in our chat (https://www.domjudge.org/chat). The current Slack has other users which also use DOMjudge in classes so maybe they might have ideas on what is the best way to share this knowledge.

@vmcj vmcj closed this Mar 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants