You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2022/11/29 12:23:27 UTC
[GitHub] [commons-bcel] nbauma109 opened a new pull request, #177: Bcelifier stackmap support verifier
nbauma109 opened a new pull request, #177:
URL: https://github.com/apache/commons-bcel/pull/177
* Bcelifier stackmap support
* BCELifier generates the stack map table so that the generated class passes the Java verifier checks.
* added some test inputs StackMapExample and StackMapExample2 compiled in JDK8
* Updated BCELifierTestCase.java: added testStackMap
* StackMapType.EMPTY_ARRAY in package visibility and updated BinaryOpCreator.java accordingly
Co-authored-by: nbauma109 <nb...@github.com>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-bcel] codecov-commenter commented on pull request #177: Bcelifier stackmap support verifier
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #177:
URL: https://github.com/apache/commons-bcel/pull/177#issuecomment-1330603035
# [Codecov](https://codecov.io/gh/apache/commons-bcel/pull/177?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#177](https://codecov.io/gh/apache/commons-bcel/pull/177?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (815a322) into [master](https://codecov.io/gh/apache/commons-bcel/commit/b015e90257850e810e57d1244664300f50de4a4c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b015e90) will **increase** coverage by `0.24%`.
> The diff coverage is `100.00%`.
```diff
@@ Coverage Diff @@
## master #177 +/- ##
============================================
+ Coverage 61.64% 61.89% +0.24%
- Complexity 3654 3684 +30
============================================
Files 363 363
Lines 15633 15700 +67
Branches 1950 1958 +8
============================================
+ Hits 9637 9717 +80
+ Misses 5122 5102 -20
- Partials 874 881 +7
```
| [Impacted Files](https://codecov.io/gh/apache/commons-bcel/pull/177?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...c/main/java/org/apache/bcel/classfile/Visitor.java](https://codecov.io/gh/apache/commons-bcel/pull/177/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2JjZWwvY2xhc3NmaWxlL1Zpc2l0b3IuamF2YQ==) | `0.00% <ø> (ø)` | |
| [src/main/java/org/apache/bcel/classfile/Code.java](https://codecov.io/gh/apache/commons-bcel/pull/177/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2JjZWwvY2xhc3NmaWxlL0NvZGUuamF2YQ==) | `73.00% <100.00%> (+1.12%)` | :arrow_up: |
| [...a/org/apache/bcel/classfile/DescendingVisitor.java](https://codecov.io/gh/apache/commons-bcel/pull/177/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2JjZWwvY2xhc3NmaWxlL0Rlc2NlbmRpbmdWaXNpdG9yLmphdmE=) | `94.40% <100.00%> (+0.12%)` | :arrow_up: |
| [...n/java/org/apache/bcel/classfile/EmptyVisitor.java](https://codecov.io/gh/apache/commons-bcel/pull/177/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2JjZWwvY2xhc3NmaWxlL0VtcHR5VmlzaXRvci5qYXZh) | `95.00% <100.00%> (+0.08%)` | :arrow_up: |
| [...n/java/org/apache/bcel/classfile/StackMapType.java](https://codecov.io/gh/apache/commons-bcel/pull/177/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2JjZWwvY2xhc3NmaWxlL1N0YWNrTWFwVHlwZS5qYXZh) | `69.76% <100.00%> (+4.76%)` | :arrow_up: |
| [src/main/java/org/apache/bcel/util/BCELifier.java](https://codecov.io/gh/apache/commons-bcel/pull/177/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2JjZWwvdXRpbC9CQ0VMaWZpZXIuamF2YQ==) | `97.28% <100.00%> (+0.85%)` | :arrow_up: |
| [.../java/org/apache/bcel/classfile/StackMapEntry.java](https://codecov.io/gh/apache/commons-bcel/pull/177/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2JjZWwvY2xhc3NmaWxlL1N0YWNrTWFwRW50cnkuamF2YQ==) | `52.48% <0.00%> (+3.86%)` | :arrow_up: |
| [.../main/java/org/apache/bcel/classfile/StackMap.java](https://codecov.io/gh/apache/commons-bcel/pull/177/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2JjZWwvY2xhc3NmaWxlL1N0YWNrTWFwLmphdmE=) | `82.05% <0.00%> (+12.82%)` | :arrow_up: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-bcel] nbauma109 commented on pull request #177: [Bcelifier] stackmap support to pass JDK verifier
Posted by "nbauma109 (via GitHub)" <gi...@apache.org>.
nbauma109 commented on PR #177:
URL: https://github.com/apache/commons-bcel/pull/177#issuecomment-1493279607
Hi @garydgregory
Please help moving forward with this PR
Thanks !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-bcel] garydgregory merged pull request #177: [Bcelifier] stackmap support to pass JDK verifier
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory merged PR #177:
URL: https://github.com/apache/commons-bcel/pull/177
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-bcel] garydgregory commented on pull request #177: Bcelifier stackmap support verifier
Posted by GitBox <gi...@apache.org>.
garydgregory commented on PR #177:
URL: https://github.com/apache/commons-bcel/pull/177#issuecomment-1330604892
@nbauma109
Rebase on git master to pick up `StackMapTest`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-bcel] nbauma109 commented on a diff in pull request #177: Bcelifier stackmap support verifier
Posted by GitBox <gi...@apache.org>.
nbauma109 commented on code in PR #177:
URL: https://github.com/apache/commons-bcel/pull/177#discussion_r1034761303
##########
src/test/resources/StackMapExample2.java:
##########
@@ -0,0 +1,8 @@
+public class StackMapExample2 {
+
Review Comment:
Commented rather than javadoc-ed as it is test input.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-bcel] nbauma109 commented on pull request #177: [Bcelifier] stackmap support to pass JDK verifier
Posted by "nbauma109 (via GitHub)" <gi...@apache.org>.
nbauma109 commented on PR #177:
URL: https://github.com/apache/commons-bcel/pull/177#issuecomment-1501405803
> @nbauma109 Would you rebase on Git master please?
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-bcel] garydgregory commented on a diff in pull request #177: Bcelifier stackmap support verifier
Posted by GitBox <gi...@apache.org>.
garydgregory commented on code in PR #177:
URL: https://github.com/apache/commons-bcel/pull/177#discussion_r1034747557
##########
src/main/java/org/apache/bcel/classfile/Code.java:
##########
@@ -247,6 +247,20 @@ public LineNumberTable getLineNumberTable() {
return null;
}
+ /**
+ * Finds the first attribute of {@link StackMap} instance.
+ * @return StackMap of Code, if it has one, else null.
+ * @since 6.7.1
+ */
+ public StackMap getStackMap() {
Review Comment:
Ah, ok, then it's the comment that needs updating IMO, as "first" makes it sound like there can be more than one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-bcel] markro49 commented on pull request #177: [Bcelifier] stackmap support to pass JDK verifier
Posted by "markro49 (via GitHub)" <gi...@apache.org>.
markro49 commented on PR #177:
URL: https://github.com/apache/commons-bcel/pull/177#issuecomment-1520949741
I compiled and ran the code and took a look at the source changes. It looks ok to me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-bcel] nbauma109 commented on a diff in pull request #177: Bcelifier stackmap support verifier
Posted by GitBox <gi...@apache.org>.
nbauma109 commented on code in PR #177:
URL: https://github.com/apache/commons-bcel/pull/177#discussion_r1034748724
##########
src/main/java/org/apache/bcel/classfile/Code.java:
##########
@@ -247,6 +247,20 @@ public LineNumberTable getLineNumberTable() {
return null;
}
+ /**
+ * Finds the first attribute of {@link StackMap} instance.
+ * @return StackMap of Code, if it has one, else null.
+ * @since 6.7.1
+ */
+ public StackMap getStackMap() {
Review Comment:
Yes. Removed "first".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-bcel] nbauma109 commented on pull request #177: Bcelifier stackmap support verifier
Posted by GitBox <gi...@apache.org>.
nbauma109 commented on PR #177:
URL: https://github.com/apache/commons-bcel/pull/177#issuecomment-1330659897
> @nbauma109 Rebase on git master to pick up `StackMapTest`.
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-bcel] garydgregory commented on pull request #177: [Bcelifier] stackmap support to pass JDK verifier
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #177:
URL: https://github.com/apache/commons-bcel/pull/177#issuecomment-1522174884
Thank you for the review @markro49, I'll merge.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-bcel] nbauma109 commented on a diff in pull request #177: Bcelifier stackmap support verifier
Posted by GitBox <gi...@apache.org>.
nbauma109 commented on code in PR #177:
URL: https://github.com/apache/commons-bcel/pull/177#discussion_r1034702240
##########
src/main/java/org/apache/bcel/classfile/StackMap.java:
##########
@@ -135,7 +135,7 @@ public StackMapEntry[] getStackMap() {
public void setStackMap(final StackMapEntry[] table) {
this.table = table != null ? table : StackMapEntry.EMPTY_ARRAY;
int len = 2; // Length of 'number_of_entries' field prior to the array of stack maps
- for (final StackMapEntry element : this.table) {
+ for (final StackMapEntry element : table) {
Review Comment:
Yes. I've just re-integrated the lost NPE fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-bcel] garydgregory commented on pull request #177: [Bcelifier] stackmap support to pass JDK verifier
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #177:
URL: https://github.com/apache/commons-bcel/pull/177#issuecomment-1501258578
@nbauma109
Would you rebase on Git master please?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-bcel] garydgregory commented on pull request #177: [Bcelifier] stackmap support to pass JDK verifier
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #177:
URL: https://github.com/apache/commons-bcel/pull/177#issuecomment-1504060136
@markro49
Any thoughts?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-bcel] garydgregory commented on a diff in pull request #177: Bcelifier stackmap support verifier
Posted by GitBox <gi...@apache.org>.
garydgregory commented on code in PR #177:
URL: https://github.com/apache/commons-bcel/pull/177#discussion_r1034692847
##########
src/test/resources/StackMapExample2.java:
##########
@@ -0,0 +1,8 @@
+public class StackMapExample2 {
+
Review Comment:
Javadoc what this tests.
##########
src/main/java/org/apache/bcel/classfile/StackMap.java:
##########
@@ -135,7 +135,7 @@ public StackMapEntry[] getStackMap() {
public void setStackMap(final StackMapEntry[] table) {
this.table = table != null ? table : StackMapEntry.EMPTY_ARRAY;
int len = 2; // Length of 'number_of_entries' field prior to the array of stack maps
- for (final StackMapEntry element : this.table) {
+ for (final StackMapEntry element : table) {
Review Comment:
This creates an NPE.
##########
src/main/java/org/apache/bcel/classfile/StackMapType.java:
##########
@@ -29,9 +29,9 @@
* @see StackMap
* @see Const
*/
-public final class StackMapType implements Cloneable {
+public final class StackMapType implements Node, Cloneable {
- public static final StackMapType[] EMPTY_ARRAY = {}; // must be public because BCELifier code generator writes calls to it
+ static final StackMapType[] EMPTY_ARRAY = {}; // package visibility as BCELifier code generator writes calls to constructor translating null to EMPTY_ARRAY
Review Comment:
You can't make something public, not public, this will break BC, you can't tell now because we are in the middle of a release.
##########
src/main/java/org/apache/bcel/classfile/Code.java:
##########
@@ -247,6 +247,20 @@ public LineNumberTable getLineNumberTable() {
return null;
}
+ /**
+ * Finds the first attribute of {@link StackMap} instance.
+ * @return StackMap of Code, if it has one, else null.
+ * @since 6.7.1
+ */
+ public StackMap getStackMap() {
Review Comment:
Should this be called getFirstStackMap()?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-bcel] nbauma109 commented on a diff in pull request #177: Bcelifier stackmap support verifier
Posted by GitBox <gi...@apache.org>.
nbauma109 commented on code in PR #177:
URL: https://github.com/apache/commons-bcel/pull/177#discussion_r1034705119
##########
src/main/java/org/apache/bcel/classfile/StackMapType.java:
##########
@@ -29,9 +29,9 @@
* @see StackMap
* @see Const
*/
-public final class StackMapType implements Cloneable {
+public final class StackMapType implements Node, Cloneable {
- public static final StackMapType[] EMPTY_ARRAY = {}; // must be public because BCELifier code generator writes calls to it
+ static final StackMapType[] EMPTY_ARRAY = {}; // package visibility as BCELifier code generator writes calls to constructor translating null to EMPTY_ARRAY
Review Comment:
Ah sorry about this. Putting it back to public.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-bcel] nbauma109 commented on a diff in pull request #177: Bcelifier stackmap support verifier
Posted by GitBox <gi...@apache.org>.
nbauma109 commented on code in PR #177:
URL: https://github.com/apache/commons-bcel/pull/177#discussion_r1034720605
##########
src/main/java/org/apache/bcel/classfile/Code.java:
##########
@@ -247,6 +247,20 @@ public LineNumberTable getLineNumberTable() {
return null;
}
+ /**
+ * Finds the first attribute of {@link StackMap} instance.
+ * @return StackMap of Code, if it has one, else null.
+ * @since 6.7.1
+ */
+ public StackMap getStackMap() {
Review Comment:
There may be at most one StackMapTable attribute in the attributes table of a Code attribute. https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-4.html#jvms-4.7.4
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-bcel] markro49 commented on pull request #177: [Bcelifier] stackmap support to pass JDK verifier
Posted by "markro49 (via GitHub)" <gi...@apache.org>.
markro49 commented on PR #177:
URL: https://github.com/apache/commons-bcel/pull/177#issuecomment-1509436320
I’m taking some time off and won’t be able to look at this for another 10
days or so.
Mark
*From:* Gary Gregory ***@***.***>
*Sent:* Tuesday, April 11, 2023 1:40 PM
*To:* apache/commons-bcel ***@***.***>
*Cc:* Mark Roberts ***@***.***>; Mention <
***@***.***>
*Subject:* Re: [apache/commons-bcel] [Bcelifier] stackmap support to pass
JDK verifier (PR #177)
@markro49 <https://github.com/markro49>
Any thoughts?
—
Reply to this email directly, view it on GitHub
<https://github.com/apache/commons-bcel/pull/177#issuecomment-1504060136>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACODEKYMQGGBLL7T4M6QPN3XAW6QHANCNFSM6AAAAAASONL6HA>
.
You are receiving this because you were mentioned.[image: Image removed by
sender.]Message ID: ***@***.***>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org