Skip to content
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

Initial Handling of Non-Void Java Method Calls in GnuCOBOL #174

Open
wants to merge 7 commits into
base: java-interop
Choose a base branch
from

Conversation

xevor11
Copy link

@xevor11 xevor11 commented Aug 23, 2024

No description provided.

xevor11 and others added 3 commits August 22, 2024 19:19
Changes mostly include:
* New option `--with{out}-java` for `configure` script
* Runtime support for calling `static void (void)` methods
* Lookup of JVM library when JAVA_HOME is set, and pre-load when
  possible
* Two preliminary tests

Co-authored-by: Nicolas Berthier <[email protected]>
Co-authored-by: Simon Sobisch <[email protected]>
cobc/codegen.c Outdated
@@ -7098,7 +7098,9 @@ output_java_call (struct cb_call *p)
char *last_dot;
char *method_name;
const char *class_name;
char method_signature[256] = "(";
Copy link
Contributor

Choose a reason for hiding this comment

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

For internal buffer sizes you have COB_*_BUFF definitions in libcob/common.h that you can use here as well. Typical signature length could exceed 256 without much effort I think (with long-enough Java package names). I'd go for COB_NORMAL_BUFF (2048) for now.

cobc/codegen.c Outdated
@@ -7115,18 +7117,103 @@ output_java_call (struct cb_call *p)
method_name = last_dot + 1;
class_name = class_and_method_name;

for(ptr = p->args; ptr != NULL; ptr = ptr->next) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: indentation is strange here. I think you need to adjust that in your settings for this project.

cobc/codegen.c Outdated
lookup_java_call(mangled);
output_line("if (call_java_%s == NULL)", mangled);
output_block_open();

output_prefix();
output_line("call_java_%s = ", mangled);
output("cob_resolve_java(\"%s\", \"%s\", \"()V\");", class_name, method_name);
output("cob_resolve_java(\"%s\", \"%s\", \"()V\");", class_name, method_name, method_signature);
Copy link
Contributor

Choose a reason for hiding this comment

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

Here you should probable replace ()V with %s so the method signature is included.

cobc/codegen.c Outdated
output_newline ();
output_prefix ();
output_line("cob_call_java(call_java_%s);\n", mangled);
output_newline();
output_block_close();
cobc_free(mangled);
Copy link
Contributor

Choose a reason for hiding this comment

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

This will likely cause a use-after-free since mangled is referenced in the list created by lookup_java_call.

Copy link
Collaborator

@GitMensch GitMensch left a comment

Choose a reason for hiding this comment

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

next bigger part here is likely adding a testcase that has a return value and a second that has parameters (or both one, for example a simple longJava.Test.addMe (int, int[]))

cobc/codegen.c Outdated
Comment on lines 7103 to 7104
char return_type_signature[32];
char method_signature[2048] = "(";
Copy link
Collaborator

Choose a reason for hiding this comment

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

use COB_MINI_BUFF and COB_NORMAL_BUFF for those and down use strncat with the _MAX defines

cobc/codegen.c Outdated
Comment on lines 7182 to 7192
cobc_err_msg(_("Unsupported array element type in Java method call"));
COBC_ABORT();
}
}
}
list_elements = inner_list;
}

if (array_dimension > 2) {
cobc_err_msg(_("Unsupported array dimension: %d"), array_dimension);
COBC_ABORT();
Copy link
Collaborator

@GitMensch GitMensch Sep 12, 2024

Choose a reason for hiding this comment

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

those two messages should be errors in cb_check_conformance as well as a check for "only field identifiers" (= no literals/functions/...) - which then also allows above to directly cast to a field pointer and operate on this; additional, for parameters passed BY REFERENCE that are a String/BigDecimal mapping item there should be a warning "immutable type implicit passed BY CONTENT".

Comment on lines +82 to +83
(*env)->ExceptionDescribe(env);
(*env)->ExceptionClear(env);
Copy link
Collaborator

Choose a reason for hiding this comment

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

fine during development, needs to be adjusted later - see review on the other PR

cobc/codegen.c Outdated
Comment on lines 7163 to 7165
case CB_TAG_INTEGER:
strcat(method_signature, "[I");
break;
Copy link
Collaborator

@GitMensch GitMensch Sep 12, 2024

Choose a reason for hiding this comment

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

In case of integer we further have to check the type per usage

pic/type java type
PIC S9(n) COMP-5, where ≥ 1 n ≤ 4 short
PIC S9(n) COMP-5, where ≥ 5 n ≤ 9 int
PIC S9(n) COMP-5, where ≥ 10 n ≤ 18 long
usage COMP-3/PACKED-DECIMAL/DISPLAY numeric java.math.BigDecimal

Note that checking the TAGs and USAGEs and CLASSes in a single switch doesn't work (check with the debugger to get more details).

Also note that PIC X (=single byte) is a special case:

  • if it has two or one level88 below it with value x'01' / x'00' --> boolean
  • otherwise: byte

For all of the types you have (after addition of the above) there needs to be a check for the occurs attribute ("more than 1 dimension" will already be checked in the conformance checks) and if existing just add [].

As the valid types are identical for both arguments and returning items, their generation should be moved out to a static helper function called for each of the parameters (the loop here) and the returning item below.

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.

3 participants